feat(redirects): serve ASO redirect overlay via api-service (Lite-E)

[alinarublea]

Implements Lite-E from the ASO dispatcher-overlay ADR (mysticat-architecture#159): move the redirect-overlay read path behind api-service instead of having Fastly sign SigV4 to S3 directly — with the per-request authorization that makes it safe (closes the cross-tenant enumeration hole rather than relocating it).

Endpoint

GET /config/:service/redirects.txt (e.g. /config/cm-p154709-e1629980/redirects.txt)

1. AuthN — inbound X-ASO-API-Key validated by AsoOverlayKeyHandler in the AUTH_HANDLERS chain (constant-time, length-safe HMAC compare; path-scoped early-bail like the GitHub webhook handler).
2. AuthZ (per request) — parse cm-p<program>-e<env> → resolve the site via the indexed Site.findByExternalOwnerIdAndExternalSiteId('p<program>','e<env>'); require the site's org to hold an ASO entitlement AND the site to be enrolled in it.
3. Reads config/<service>/redirects.txt from S3_ASO_OVERLAYS_BUCKET using the Lambda's own execution role.
4. Returns text/plain (200). Every authz miss → an indistinguishable 404 (no signal of which programs exist/are entitled). 400 for a malformed service id; 500 only for unexpected failures.

Why this is the safe Lite-E

The shared X-ASO-API-Key alone would let any key-holder enumerate every program's overlay by walking cm-pX-eY. The per-request entitlement check resolves only provisioned, ASO-entitled tenants — enumeration is gone, and authz is server-side in code we own and test. (Isolation ceiling is still Option A per the ADR; Full-E = swap the static key for a consumer TA, separate follow-up.)

Notable details

• :env dropped from the route — the bucket is already env-scoped via the Lambda's own deployment, so the segment was dead. Edge/dispatcher URL change to be coordinated separately with the dispatcher side.
• 403 → 404 mapping: the reader role intentionally has no s3:ListBucket (least privilege, no S3-layer key enumeration), so a missing object surfaces as 403 AccessDenied not 404. Mapped to 404 for the caller but logged at error so a missing IAM grant (spacecat-infrastructure#620) stays alertable.
• Route is in INTERNAL_ROUTES (excluded from S2S), authenticated by the key handler — no change to the global handler chain semantics beyond adding AsoOverlayKeyHandler.

⚠️ Required companion changes (not in this PR)

1. Lambda IAM grant — spacecat-infrastructure#620 grants spacecat-role-lambda-generic s3:GetObject on spacecat-*-aso-overlays/config/*. Land it before this deploys per env.
2. Fastly — re-point the /config/* origin from S3 to the api-service backend (Fastly-admin; supersedes the feat: limiting scrape to 100 urls per message #617 signing snippet) and drop :env from the dispatcher URL.
3. Secrets — S3_ASO_OVERLAYS_BUCKET + ASO_OVERLAY_API_KEY in Vault per env + npm run deploy-secrets.

Supersedes spacecat-infrastructure#616 and #617 (no hand-rolled SigV4 in VCL, no long-lived static IAM keys to Fastly).

Tests

• test/controllers/redirects.test.js — rewritten for the authz flow (200, 400, 404 for no-site / not-entitled / not-enrolled, NoSuchKey→404, AccessDenied→404, 500).
• test/support/aso-overlay-key-handler.test.js — the key handler.
• test/routes/index.test.js + auth-handlers-order updated.
• Integration test is self-contained: seeds throwaway entitled sites + ASO enrollments via PostgREST in before()/after() so the count-asserted global fixtures are untouched.
• npm run lint clean; full unit suite green (12,245 passing; the one parallel-run timeout is a pre-existing plg-onboarding hook flake — passes 392/392 in isolation).

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

This PR will trigger a minor release when merged.

[MysticatBot]

Hey @alinarublea,

Verdict: Request changes - two small fixes needed before this is ready.
Changes: adds a new GET /config/:env/:service/redirects.txt endpoint that serves ASO redirect overlays from S3, authenticated by a static API key validated in the controller (8 files).

Must fix before merge

1. [Important] safeEqual leaks API key length via timing side-channel on length mismatch - src/controllers/redirects.js:43 (details inline)
2. [Important] :env path parameter is validated but never used in S3 key construction - src/controllers/redirects.js:113 (details inline)

Non-blocking (5): minor issues and suggestions

• suggestion: Add log.warn('[aso-overlay] auth failed', { hasKey: !!providedKey }) before the 401 return so repeated failures produce an observability signal for abuse detection - src/controllers/redirects.js:91
• suggestion: Tighten the ANONYMOUS_ENDPOINTS regex service segment from [^/]+ to cm-p\d+-e\d+ to match the controller's own validation (defense-in-depth) - src/support/access-control-util.js:29
• nit: Cap SERVICE_RE digit sequences to realistic lengths, e.g. /^cm-p\d{1,10}-e\d{1,10}$/, to prevent arbitrarily long S3 key lookups - src/controllers/redirects.js:25
• suggestion: Add a test case for the err.$metadata?.httpStatusCode === 404 fallback branch (currently only err.name === 'NoSuchKey' is exercised) - test/controllers/redirects.test.js
• suggestion: Add a test for the new anonymous endpoint regex in access-control-util.test.js to verify the auth bypass boundary matches expectations - src/support/access-control-util.js:29

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 5m 28s | Cost: $6.26 | Commit: 3a68cd5bee536c9c972e598f233b871d2afde8e2
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

issue (blocking): safeEqual returns early on length mismatch (line 46-48), creating a timing oracle that leaks the configured API key length. The current JSDoc claims "without a timing side channel" which is inaccurate - the early return IS a measurable timing difference. An attacker measuring response times over many requests can binary-search the key length.

Fix: Hash both inputs to a fixed length before comparing:

function safeEqual(a, b) {
  if (typeof a !== 'string' || typeof b !== 'string') {
    return false;
  }
  const ha = crypto.createHmac('sha256', 'aso-key-compare').update(a).digest();
  const hb = crypto.createHmac('sha256', 'aso-key-compare').update(b).digest();
  return crypto.timingSafeEqual(ha, hb);
}

This eliminates the length oracle at negligible cost. The HMAC key is not a secret here - it just normalizes both inputs to 32 bytes regardless of input length.

[MysticatBot]

issue (blocking): The :env path parameter is validated against the ENVS allowlist (line 84) but is never used in the S3 key. The key is always config/${service}/redirects.txt regardless of whether the caller requests /config/dev/... or /config/prod/....

Since S3_ASO_OVERLAYS_BUCKET is per-deployment (one bucket per env), this is functionally correct today - but the validated-yet-unused parameter is confusing. A future maintainer will reasonably assume it is a bug.

Fix (pick one):

1. Add a one-line comment above the key construction explaining the design: the env in the URL is for Fastly routing compatibility only; the backend reads from whichever bucket its deployment is configured with.
2. Or, if the intent is that reqEnv should match the deployment env, assert reqEnv === env.ENVIRONMENT and return 400 on mismatch.
3. Or, if the bucket is ever shared across envs, incorporate env into the key: config/${reqEnv}/${service}/redirects.txt

[MysticatBot]

Hey @alinarublea,

Verdict: Request changes - one convention gap remains after an otherwise clean fix-up.
Changes: adds a new GET /config/:env/:service/redirects.txt endpoint serving ASO redirect overlays from S3, authenticated by a static API key validated in the controller (11 files).

Must fix before merge

1. [Important] Missing integration tests per CLAUDE.md requirement - test/it/ (details inline)

Non-blocking (1): minor issues and suggestions

• nit: ANONYMOUS_ENDPOINTS regex uses unbounded \d+ while the controller's SERVICE_RE uses \d{1,10} - align for defense-in-depth consistency - src/support/access-control-util.js:30

Previously flagged, now resolved

• safeEqual timing side-channel: now HMAC-normalizes both inputs to fixed 32-byte digests before timingSafeEqual
• :env path parameter unused: now validates requested env against the deployment bucket's env, returning 404 on mismatch
• Auth failure logging added
• ANONYMOUS_ENDPOINTS regex tightened from [^/]+ to cm-p\d+-e\d+
• SERVICE_RE digit runs capped to {1,10}
• Test for $metadata.httpStatusCode === 404 fallback added
• Access-control-util boundary test added

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 2m 6s | Cost: $4.43 | Commit: 473e9d42b83d3e79bcbc688a5fdd2e2fd2a2b393
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

issue (blocking): The repo CLAUDE.md explicitly requires: "New or modified endpoints must include integration tests in test/it/". This PR adds a new endpoint but only includes unit tests.

The IT framework is PostgreSQL-focused and this endpoint is S3-only, so you may need to extend the harness or document why ITs are deferred (e.g. no S3/LocalStack support in the current IT infrastructure). If deferring, a follow-up ticket and a code comment noting the gap would satisfy the intent.

[MysticatBot]

nit: The regex here uses unbounded \d+ while SERVICE_RE in the controller uses \d{1,10}. The controller is the enforcement boundary so this is not a security gap, but aligning them (cm-p\d{1,10}-e\d{1,10}) keeps defense-in-depth consistent and matches the stated intent from the commit message.

[MysticatBot]

Hey @alinarublea,

Verdict: Request changes - OpenAPI spec is out of sync with the implemented route.
Changes: adds a new GET /config/:service/redirects.txt endpoint serving ASO redirect overlays from S3, now with per-tenant authz (site resolution + entitlement + enrollment check) and HMAC-normalized constant-time key validation (15 files).

Must fix before merge

1. [Important] OpenAPI spec declares a path that does not exist in the router - docs/openapi/api.yaml:309, docs/openapi/aso-overlay-api.yaml:18-25 (details inline)

Non-blocking (1): minor issues and suggestions

• nit: .env.example:82 comment says GET /config/<env>/<service>/redirects.txt but the route has no <env> segment - update when fixing the spec.

Previously flagged, now resolved

• safeEqual timing side-channel: now HMAC-normalizes both inputs to fixed 32-byte digests before timingSafeEqual
• :env path parameter unused: removed :env from the route entirely
• Missing integration tests: full IT suite added (test/it/postgres/redirects.test.js + shared test factory)
• Auth failure logging added
• ANONYMOUS_ENDPOINTS regex tightened
• SERVICE_RE digit runs capped to {1,10}
• Test for $metadata.httpStatusCode === 404 fallback added
• Access-control-util boundary test added

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 2m 27s | Cost: $5.38 | Commit: 01c69fda1f323fe6de073fbf66b718ba967bdef1
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

issue (blocking): The OpenAPI spec defines the path as /config/{env}/{service}/redirects.txt with an env path parameter (enum: [dev, stage, prod]), but the actual implementation route is GET /config/:service/redirects.txt - a single path parameter with no {env} segment.

This violates the repo CLAUDE.md rule: "Specification Sync: Keep OpenAPI specs and implementation in sync."

The auth handler regex (OVERLAY_ROUTE), controller params destructuring ({ service }), INTERNAL_ROUTES entry, and all integration tests confirm the route has no env segment. The spec is stale from before :env was removed.

Fix: Remove the env parameter from aso-overlay-api.yaml, update the path in api.yaml to /config/{service}/redirects.txt, and fix the .env.example comment. Run npm run docs:lint after.

[MysticatBot]

Hey @alinarublea,

Verdict: Approve - all prior findings addressed, clean on re-review.
Changes: adds a new GET /config/:service/redirects.txt endpoint serving ASO redirect overlays from S3, with per-tenant authz (site resolution + entitlement + enrollment) and HMAC-normalized constant-time key validation (15 files).

Previously flagged, now resolved

• safeEqual timing side-channel: now HMAC-normalizes both inputs to fixed 32-byte digests before timingSafeEqual
• :env path parameter unused: removed from route entirely
• Missing integration tests: full IT suite added (test/it/postgres/redirects.test.js + shared test factory)
• OpenAPI spec out of sync with route: synced in commit 7e7caed (path now /config/{service}/redirects.txt, env parameter removed)
• Auth failure logging added
• ANONYMOUS_ENDPOINTS regex tightened to match controller's capped digit pattern
• SERVICE_RE digit runs capped to realistic lengths
• Test for $metadata.httpStatusCode === 404 fallback added
• Access-control-util boundary test added
• .env.example comment updated to match the env-less route

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 17s | Cost: $6.50 | Commit: 7e7caed20f1f8de6a7fb0bae278157a9ce453d3a
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Hey @alinarublea,

Verdict: Approve - all prior findings addressed, clean on re-review.
Changes: adds two defensive test cases to the ASO overlay key handler verifying graceful null returns on non-string header values and missing pathInfo fields (1 file).

Previously flagged, now resolved

• safeEqual timing side-channel: now HMAC-normalizes both inputs to fixed 32-byte digests before timingSafeEqual
• :env path parameter unused: removed from route entirely
• Missing integration tests: full IT suite added (test/it/postgres/redirects.test.js + shared test factory)
• OpenAPI spec out of sync with route: synced (path now /config/{service}/redirects.txt, env parameter removed)
• Auth failure logging added
• ANONYMOUS_ENDPOINTS regex tightened to match controller's capped digit pattern
• SERVICE_RE digit runs capped to realistic lengths
• Test for $metadata.httpStatusCode === 404 fallback added
• Access-control-util boundary test added
• .env.example comment updated to match the env-less route

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 0m 33s | Cost: $2.67 | Commit: 0a4623b70ac7a33f6824a7fa779afb810f04f573
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[solaris007]

🎉 This PR is included in version 1.589.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀