Skip to content

fix(ci): openapi-security-tiers must read ALWAYS_PROTECTED_API_PATTERNS (base-red #12581) - #12652

Merged
diegosouzapw merged 1 commit into
release/v3.8.51from
fix/release-basereds-gate
Sep 5, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.51from
fix/release-basereds-gate

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

⚠️ base-red inherited: #12581

The failure

release/v3.8.51 is red on check:openapi-security-tiers with four routes reported as unprotected:

POST /api/providers/{id}/claude-auth/apply-local: has x-always-protected but is NOT in ALWAYS_PROTECTED_API_PATHS [...]
POST /api/providers/{id}/claude-auth/export:      ...
POST /api/providers/{id}/codex-auth/apply-local:  ...
POST /api/providers/{id}/codex-auth/export:       ...

They are not unprotected

isAlwaysProtectedPath() ORs two arrays:

// src/server/authz/routeGuard.ts
ALWAYS_PROTECTED_API_PATHS.some((p) => path === p || path.startsWith(p)) ||
ALWAYS_PROTECTED_API_PATTERNS.some((re) => re.test(path))

All four routes are covered by the pattern that shipped with the credential-export hard-gate:

/^\/api\/providers\/[^/]+\/(claude|codex)-auth\/(export|apply-local)\/?$/

A runtime probe over the real isAlwaysProtectedPath() returns true for all four. The gate parsed only ALWAYS_PROTECTED_API_PATHS and never ALWAYS_PROTECTED_API_PATTERNS, so it was blind to half of its own input — a false positive, not a security hole.

This is the residual half of the defect #12350 fixed for the LOCAL_ONLY tier this cycle; that arm already reads both LOCAL_ONLY_API_PREFIXES and LOCAL_ONLY_API_PATTERNS.

Change

  • Parse ALWAYS_PROTECTED_API_PATTERNS, and include it in the fatal parse guard so a future formatting change in routeGuard.ts fails loudly instead of silently degrading back into false positives.
  • Add coveredByAlwaysProtected() mirroring coveredByLocalOnly() — both concretize {param} before matching.
  • Name both arrays in the error text.

Validation

Gate on a clean tree: FAIL — 4 annotation mismatches before, PASS — all security tier annotations match routeGuard.ts (exit 0) after.

New tests/unit/openapi-security-tiers-gate.test.ts runs the gate as a subprocess — deliberately not importing routeGuard.ts, so the unit suite gains no DB handle (that suite is already hanging on one). Mutation-checked: reverting the gate to its previous version makes the test fail and print all four routes; restoring the fix makes it pass.

@diegosouzapw

Copy link
Copy Markdown
Owner Author

The red No new ESLint warnings check on this PR is inherited from the base, not from this diff.

It fails on one error in src/app/(dashboard)/dashboard/combos/page.tsx:774 (react-hooks/set-state-in-effect) — a file this PR does not touch. That is the same ESLint errors: 1 error(s) that base-red #12581 reports, and it is fixed in #12671.

This PR changes only scripts/check/check-openapi-security-tiers.mjs (globally ignored by ESLint) and adds one test file, which lints clean.

…cuting gate test (#12581)

The ALWAYS_PROTECTED two-arm read itself landed in #12605; what was still
missing is a regression guard. This runs the real gate and asserts it exits 0
with no "NOT covered" line, so the LOCAL_ONLY-arm defect (#12350) cannot
silently reappear on the ALWAYS_PROTECTED arm. Also fails the parse guard when
ALWAYS_PROTECTED_API_PATTERNS comes back empty, instead of reporting every
regex-covered route as an annotation mismatch.
@diegosouzapw
diegosouzapw force-pushed the fix/release-basereds-gate branch from d9b70d3 to 514d3c3 Compare September 5, 2026 06:14
@diegosouzapw
diegosouzapw merged commit ec4f951 into release/v3.8.51 Sep 5, 2026
6 of 11 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…cuting gate test (diegosouzapw#12581) (diegosouzapw#12652)

Merged as a reduced diff, and worth recording why.

The two-arm `ALWAYS_PROTECTED` read this PR proposed had already landed in diegosouzapw#12605 while this branch was open — the tip carries `coveredByAlwaysProtected()` with both arms and the new error wording. I ran the gate on the current tip to be sure: `PASS — all security tier annotations match routeGuard.ts`. Merging the whole branch would have reintroduced the same logic under a different comment.

What was genuinely missing, and is what merged:

- **`tests/unit/openapi-security-tiers-gate.test.ts`** — executes the real gate and asserts exit 0 with no "NOT covered" line. diegosouzapw#12605 fixed the defect but left no guard, so the LOCAL_ONLY-arm bug (diegosouzapw#12350) could reappear on the ALWAYS_PROTECTED arm exactly as it did the first time. 1/1 green.
- **The parse guard** — `ALWAYS_PROTECTED_PATTERNS.length === 0` now fails the constant-parse check with its own count in the message. Without it, a regex array that stops parsing degrades into "every pattern-covered route is an annotation mismatch" instead of saying so.

A note for the record: my first read of this PR was wrong. I ran the gate in the main checkout, which was 11 commits behind `origin/release/v3.8.51`, saw the pre-diegosouzapw#12605 failure, and classified this as fixing a live red. It was not — the checkout was stale. Corrected before anything was merged.
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