Skip to content

feat(auth): support overlapping proxy secret rotation - #2

Closed
julianknutsen wants to merge 2 commits into
gascity-ssofrom
ai/proxy-auth-dual-secret
Closed

julianknutsen wants to merge 2 commits into
gascity-ssofrom
ai/proxy-auth-dual-secret

Conversation

@julianknutsen

@julianknutsen julianknutsen commented Jul 15, 2026 •

Copy link
Copy Markdown

Summary

Add a bounded current-plus-previous acceptance window for rotating the HyperDX proxy-auth shared secret without mismatching old and new Caddy injectors during convergence.

  • introduce optional PROXY_AUTH_SHARED_SECRET_PREVIOUS
  • keep PROXY_AUTH_SHARED_SECRET mandatory whenever proxy auth is enabled
  • fail closed in middleware when neither value is configured
  • reject a nonempty previous value that equals the current value
  • update the downstream fork contract and the image patch guard
  • add a secretless, read-only pull_request auth quality gate for the Gas City branch
  • replace inherited secret-bearing pull_request_target AI reviewers with permissionless, manual-only disabled stubs

Security properties

  • The request value is compared against both configured slots; both comparisons are evaluated before the result is returned.
  • Each nonempty comparison uses Node crypto.timingSafeEqual, with a length check to avoid unequal-length exceptions.
  • Missing, empty, wrong, and unequal-length request values are rejected.
  • Proxy-disabled behavior remains stock; proxy-enabled startup still requires the current value even if a previous value exists.
  • Rejected credential values are not logged, included in startup errors, or embedded by this change.
  • Image publication now depends on the verification job, and the symbol guard checks exact production JavaScript files rather than the broader build tree that also contains compiled tests.

Verification

  • TDD RED before implementation: 5 expected failures in the two focused suites.

  • Focused proxy-auth/config Jest after review fixes: 27/27 passed.

  • API ESLint: passed.

  • API TypeScript tsc --noEmit: passed.

  • API production build and exact compiled production-symbol checks: passed.

  • make ci-lint: all 4 projects passed.

  • make ci-unit: CLI 8 passed; common-utils 1,169 passed; app 1,752 passed with 9 existing skips.

  • actionlint, YAML parse, lint-staged, staged credential-pattern scan, and git diff --check: passed.

  • Independent security/correctness review: approved after non-vacuous parsing tests, retirement coverage, restart documentation, and the verify-before-build dependency were added.

  • Exact local gascity-pr command path: immutable install, API lint, API typecheck, focused Jest 27/27, production build, and all compiled-symbol guards passed.

  • Final independent GHA threat review: approved; no PR-triggered secret path, write/comment path, mutable privileged checkout, or publish capability remains.

How to test on Vercel preview

N/A — non-UI change.

Deployment ordering — STOP-gated

Hard dependency: gascity/infra#1232 must be merged and deployed on every identity Caddy node first. Before introducing any fresh credential, prove each effective Caddy ExecStart omits --environ and complete the prerequisite canaries. A merge alone is not sufficient.

After that prerequisite is live, use an attended rollout:

  1. Add and seed the previous-value ExternalSecret/OpenBao property so reconciliation is stable; do not print either value.
  2. Atomically set HyperDX current=new and previous=old, force ESO reconciliation, and verify Secret propagation without exposing plaintext.
  3. Deploy/restart every HyperDX API instance on this patched image, then prove readiness and that both old and new values are accepted.
  4. Serially converge every Caddy injector to new, proving each node before advancing. HyperDX accepts both values during this convergence.
  5. Prove all injectors use new without printing it.
  6. Clear previous, reconcile, deploy/restart every HyperDX API instance again, then prove new is accepted and old is rejected before revoking old.

The values are captured at module load, so changing the Kubernetes Secret alone does not update a running API process.

Availability boundary

The current HyperDX deployment is one replica with Recreate. Its embedded alert scheduler has no proven leader election, so overlapping API replicas are not currently safe. The API replacement in step 3 and the retirement replacement in step 6 each cause a bounded interruption. Dual-secret acceptance prevents credential mismatch during Caddy convergence; it does not make those two API restarts zero-downtime.

Rollback

  • During the overlap window, serially restore any updated Caddy injector to old; the patched backend still accepts old through the previous slot.
  • If the patched image or first replacement fails, atomically restore the prior secret state (current=old, previous empty), reconcile it, and repin/restart the prior image before resuming traffic.
  • Never roll Caddy back to an ExecStart containing --environ.
  • After old is retired or revoked, do not re-enable it. Generate a new replacement credential and repeat the reviewed overlap procedure.

This PR performs code, documentation, and CI workflow changes only. It does not mutate OpenBao, Kubernetes, Caddy, Cloudflare, GHCR tags, or any live system.

Test User added 2 commits July 15, 2026 23:25
Accept a distinct previous value during attended rotations while keeping the current value mandatory and the middleware fail-closed. Tighten the production artifact guard and prevent image publication before verification succeeds.
Run auth verification for ordinary pull requests without secrets or write credentials. Replace inherited pull_request_target AI reviewers with visibly disabled manual stubs until a separately reviewed isolation design exists.
@julianknutsen

Copy link
Copy Markdown
Author

Superseded by #4, the security rebase of gascity-sso onto @hyperdx/app@2.42.0.

This PR's content is carried forward there, with credit to this PR:

  • 87dde18a feat(auth): support overlapping proxy secret rotation → f567cb14. Its tests moved to the fork-owned __tests__/proxyAuthConfig.test.ts so that upstream's config.test.ts stays byte-identical.
  • f09be641 ci: add safe Gas City PR quality gate → a38ca3bc. The two workflow stubs became deletions of every inherited workflow, and the symbol-guard paths were fixed for 2.42's flat API build.

Rotating with the previous-secret overlap window is now required after the 2.42 deploy, because #4 fixes a leak of the shared secret into request logs. This PR is what makes that rotation mismatch-free. The ai/proxy-auth-dual-secret branch is left in place.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant