Security / performance / scalability hardening (PRD-68) - #93
Merged
Conversation
…68 P1) - yamlQuote now emits a single fully-escaped YAML scalar (single-quoted for plain-printable inline JSON, otherwise JSON-encoded); toYAMLStringList JSON-encodes elements. Every user-derived template sink (HF token, model id, S3 URI, kv-cache dtype, revision, bucket, prefix, region, cache id) goes through it in all six templates. - Generated model pods always set allowPrivilegeEscalation=false and drop the SA token unless the streamer SA is needed. - New internal/api/validate.go: strict charset/enum validation for every free-text field on run / suite / model-cache / register requests, wired into the four create paths (400 with a field-specific message). - Typed manifest decode uses DisallowUnknownFields (orchestrator applyYAML and cache-job apply). - Tests: validator table tests + a render test proving a crafted hf_token cannot add securityContext.privileged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Jeremy Cowan <jicowan@hotmail.com>
…, scraper cost (PRD-68 P2) - waitForReady listed pods with "app=<name>" but the template labels app.kubernetes.io/name — the OOM scan matched nothing, so every OOM burned the full 25-minute readiness timeout. Fixed the selector. - oom.Detector gains CheckPodObject: readiness loops pass the pod they already listed (no Pods.Get per pod per tick) and the Events.List is only issued when a container has terminated or restarted. - client-go QPS/Burst raised from the 5/10 default to 50/100 (K8S_CLIENT_QPS / K8S_CLIENT_BURST) so a dozen concurrent runs no longer client-side-throttle teardown behind readiness polls. - pgxpool configured explicitly: MaxConns 10 / MinConns 2 (DB_MAX_CONNS, DB_MIN_CONNS), idle/lifetime bounds, statement_timeout 30s (DB_STATEMENT_TIMEOUT_SECONDS), application_name. - Host-memory scraper samples every 15s instead of 5s and caches the pod-name set instead of a Pods.List per sample (re-resolves on roll). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Jeremy Cowan <jicowan@hotmail.com>
…cing poller, cross-pod delete (PRD-68 P3) Admission: Execute/ExecuteSuite take a per-pod slot (MAX_CONCURRENT_RUNS, default 4) after registering for cancel and before touching the cluster; queued runs stay pending and cancellable. A catalog seed no longer fans the whole matrix out to Karpenter at once. Distributed runs wait for the shared pool lock (DISTRIBUTED_LOCK_WAIT, default 30m) instead of failing. Coordination: one poller per pod reads owner_pod/cancel_requested/status for every owned run in a single query every 5s and cancels runs that were cancelled, DELETED, re-claimed by another pod (fenced), or already moved to a terminal status by recovery. Replaces the per-run pollers. Terminal status writes are conditional (WHERE status IN pending/running) and return ErrRunNotActive so a stale owner can never overwrite recovery's outcome. Runs and suites are inserted with owner_pod set. Delete: an active run/suite delete now sets the DB cancel flag before removing the row; the owner's poller treats the missing row as cancel and drives normal teardown (previously a cross-pod delete orphaned the model Deployment + loadgen Job until natural completion). Recovery: orphan recovery, catalog MV refresh run under a Postgres advisory lock so one replica does the work; orphaned runs are salvaged from the S3 summary (completed + cost) when the loadgen already finished, else failed. Shutdown: SIGTERM -> http.Server.Shutdown, then Drain waits up to SHUTDOWN_GRACE (10m) for in-flight runs while heartbeat + poller keep running; Helm sets terminationGracePeriodSeconds 660, a preStop sleep, readiness on /readyz (DB ping + not draining), and a PDB (minAvailable 1). /metrics exposes run/queue/recovery/fencing gauges in Prometheus text. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Jeremy Cowan <jicowan@hotmail.com>
- Migration 045: benchmark_runs.created_by / test_suite_runs.created_by (Cognito sub of the submitter), model_cache.owner_pod, and partial indexes for the active-owner orphan scans + created_by. - Cancel and delete of runs/suites are owner-or-admin (reads stay open to every role per PRD-48; legacy NULL created_by rows stay mutable). - Model-cache rows are owned by the watching pod; a 60s-grace/30s recovery loop (advisory-locked) adopts 'caching' rows whose pod stopped heartbeating, re-attaches a watcher, and fails rows whose Job is gone. Previously a pod restart mid-cache stranded the row in 'caching' forever and blocked re-caching via the 409. - /model-cache gains ?hf_id= for point lookups (used by P5 frontend work). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Jeremy Cowan <jicowan@hotmail.com>
…utes (PRD-68 P5)
Backend
- FetchModelConfig serves repeats from a 10m in-process cache; the platform
HF token is cached 5m in secrets.Manager (invalidated on write); the
host-memory calibration aggregate is cached 60s. /recommend no longer
costs an S3/HF round-trip + Secrets Manager call + full-table percentile
per slider tick.
- One shared S3 client (recommend.SharedS3Client) replaces
LoadDefaultConfig-per-call in the recommender and the results reader.
- ListJobs: page + COUNT as two statements in one pgx batch instead of
COUNT(*) OVER () on the materialized UNION, so the created_at indexes
are usable again. ListSuiteRunsWithNames is paginated {rows,total}
(was a bare array hard-capped at 100).
- DashboardStats: counts + lifetime cost in one pass; the 14-day series
now carries runs/suites per day so the Dashboard chart needs no list
fetch; the per-day scan is bounded to the last 14 days.
- GetBenchmarkRun joins model/instance names so the polled run detail is
one query; suite detail loads scenario overrides once (was N+1).
- Response cache sweeps expired entries every 60s and caps at 10k; the
pricing region key is validated against an AWS region pattern.
- Orphan reap short-circuits with one Nodes.List when no multinode nodes
exist (was NodePools.List + per-pool EC2NodeClass.Get + Nodes.List every 30s).
Frontend
- React.lazy for every route except Login/Dashboard: initial chunk
893 KB -> 215 KB.
- Dashboard fetches 8 runs + 3 suites (was 100 + all); Run page and combobox
use ?hf_id=/status= point lookups; status poll pauses in hidden tabs.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Jeremy Cowan <jicowan@hotmail.com>
…rd, Karpenter limits, retention, auth rate limit (PRD-68 P6) - Resource names embed 12 chars of the run id (was 8 → 32-bit collision space); every generated Deployment/Service/Job/ConfigMap carries accelbench/run-id. The suite loadgen Jobs now actually get the suite-run-id label CleanupSuiteResources selects on (it was never set, so suite loadgen cleanup matched nothing). - Leak reconciler (5m, advisory-locked): deletes accelbench/role objects whose run row is terminal or gone after a 10m grace; legacy unlabelled objects only once older than any possible run. - docker/migrate.sh keeps a schema_migrations ledger, skips applied files, and runs psql with ON_ERROR_STOP so a failing statement fails the Job. - Helm: ingress.scheme (internet-facing|internal); the chart refuses to render cognito.authDisabled=true on an internet-facing ALB unless ingress.allowAuthDisabled=true. api.runRetentionDays wires RUN_RETENTION_DAYS. - Terraform (NOT applied — plan first): nvidia.com/gpu, aws.amazon.com/neuron and memory limits on the gpu/neuron/cpu NodePools (variables with defaults 64/64/8000Gi); opt-in cognito_advanced_security_mode. - Opt-in retention: RUN_RETENTION_DAYS purges terminal runs/suites (and metrics) older than N days hourly in batches; default 0 = off. - Per-IP token-bucket limiter (30/min, burst 10) on the public /auth/* routes; 429 + Retry-After. - 35 handler sites that returned raw err.Error() on 5xx now log the detail and return an opaque message. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Jeremy Cowan <jicowan@hotmail.com>
… P6) 001_initial.sql uses bare CREATE TABLE, so the first strict replay against an existing database failed. When the ledger is empty and benchmark_runs exists, replay once the old lenient way and back-fill schema_migrations; every later run is strict (ON_ERROR_STOP). Verified live on accelbench-eks. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Jeremy Cowan <jicowan@hotmail.com>
Signed-off-by: Jeremy Cowan <jicowan@hotmail.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…eQL) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Jeremy Cowan <jicowan@hotmail.com>
…ger-conversion) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Jeremy Cowan <jicowan@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes every confirmed finding from the three-way security / performance / scalability review (PRD-68). Six phases, one commit each, plus a migration-script fix found during deploy.
Security
yamlQuoteemits a fully escaped scalar; every user-derived template sink goes through it; typed decode usesDisallowUnknownFields; generated pods setallowPrivilegeEscalation: false. Newinternal/api/validate.gorejects malformed model ids, S3 URIs, tags, tokens, enums (400).created_by, migration 045): cancel/delete are owner-or-admin. Helm refusescognito.authDisabled=trueon an internet-facing ingress. Per-IP rate limit on/auth/*. 35 handlers stop returning rawerr.Error()on 5xx.Lifecycle / scalability
MAX_CONCURRENT_RUNS, default 4); queued runs staypendingand cancellable. Distributed runs wait for the pool lock instead of failing.ErrRunNotActive).owner_podset at insert.SHUTDOWN_GRACE10m, Helm grace 660s, preStop, PDB,/readyz,/metrics). A rolling restart no longer kills in-flight runs.app=→app.kubernetes.io/name=, was dead code), client-go QPS 50/100, pgxpool sizing +statement_timeout.accelbench/run-idlabels + leak reconciler; the suitesuite-run-idlabel is now actually set (was selected on but never emitted).migrate.shkeeps aschema_migrationsledger (lenient bootstrap for pre-ledger DBs, strict thereafter). Karpenter GPU/Neuron/memory limits + opt-in Cognito threat protection in Terraform (not applied). Opt-inRUN_RETENTION_DAYSpurge.Performance
ListJobspage+count batch; dashboard per-day series server-side;GetBenchmarkRunjoins names; suite overrides loaded once; response cache sweep + cap;/suite-runspaginated.React.lazyroutes: initial chunk 893 KB → 215 KB. Dashboard fetches 8 runs + 3 suites; point lookups for cached models; status poll pauses in hidden tabs.Live validation (accelbench-eks, 2026-09-02)
/readyz/metrics, region validation, lazy bundleGPU path not re-validated (g5/g6 ICE in all us-east-2 AZs during the window); used the r8g CPU tier. Pre-existing and out of scope: CPU-tier runs persist NULL
total_cost_usd; RDS secret rotation is enabled again after the Aurora rebuild.Test plan
go test ./... && go vet ./...cd frontend && npx tsc --noEmit && npm run build && npx vitest runhelm lint+ fail-guard render check;terraform validate🤖 Generated with Claude Code