fix(auth): require an access-token type on the OIDC bearer path (supersedes #1077) - #1078
fix(auth): require an access-token type on the OIDC bearer path (supersedes #1077)#1078seonghobae wants to merge 8 commits into
Conversation
The OIDC verifier accepted any RS256 token whose signature, issuer, and audience matched, without checking token type. Since an OIDC ID token carries aud == client_id, a frontend ID token could be replayed as an API access token (RFC 8725 §3.11 / Strix HIGH, CVSS 8.8). naruon's API credential is the OIDC access token (the frontend sends token_response.access_token). Require the token to be marked as an access token — Keycloak body typ="Bearer" or RFC 9068 header typ="at+jwt" — and reject ID-token material (Keycloak typ="ID") and unmarked tokens. Regression tests replay an ID-token-shaped token and assert 401; they fail on the prior code and pass here. OIDC access-token fixtures updated to carry the realistic typ="Bearer". Supersedes #1077, whose inverse approach rejected scope-bearing Keycloak access tokens (breaking SSO login) while still admitting ID tokens. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
PR governance metadata gate is not ready for
|
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Backend (3 files)"]
S1 --> I1["API and service runtime"]
I1 --> R1["Review risk: Backend (3 files)"]
R1 --> V1["backend tests"]
|
…ccess-token-type # Conflicts: # backend/tests/test_repo_hygiene.py # docker-compose.infra.yml # frontend/screenshot.cjs
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head5611121bcc9d21cf0cb551c18a8d6917c5c00f3d. -
Head SHA:
5611121bcc9d21cf0cb551c18a8d6917c5c00f3d -
Workflow run: 29561805402
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Backend (3 files)"]
S1 --> I1["API and service runtime"]
I1 --> R1["Review risk: Backend (3 files)"]
R1 --> V1["backend tests"]
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head5611121bcc9d21cf0cb551c18a8d6917c5c00f3d. -
Head SHA:
5611121bcc9d21cf0cb551c18a8d6917c5c00f3d -
Workflow run: 29561805402
-
Workflow attempt: 2
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Backend (3 files)"]
S1 --> I1["API and service runtime"]
I1 --> R1["Review risk: Backend (3 files)"]
R1 --> V1["backend tests"]
|
current HEAD 보강 ( |
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head02d14359366562d6e00035c3a76aec55b0b2962f. -
Head SHA:
02d14359366562d6e00035c3a76aec55b0b2962f -
Workflow run: 29744545421
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Backend (3 files)"]
S1 --> I1["API and service runtime"]
I1 --> R1["Review risk: Backend (3 files)"]
R1 --> V1["backend tests"]
|
Current-head audit ( Central fix: ContextualWisdomLab/.github#579 at The existing current-head REQUEST_CHANGES remains valid until #579 merges and a fresh central review proves the hosted path. Cross-repository status publication also remains fail-closed pending the configured central write token. |
02d1435 to
af8d471
Compare
|
Superseded by #1123 — closing to keep the queue clean. #1123 (author-driven security follow-up) remediates the same Strix finding ("OIDC ID tokens accepted as API bearer sessions without access-token type enforcement") with a strictly broader contract: exact RFC 9068 If #1123 stalls, this PR can be reopened as the fallback fix. |
What
Fix the Strix HIGH (CVSS 8.8) "OIDC ID tokens accepted as API bearer sessions without token-type validation" in
backend/api/auth.py, and add regression tests.The OIDC verifier accepted any RS256 token whose signature, issuer, and
audmatchedOIDC_CLIENT_ID, with no token-type check. An OIDC ID token carriesaud == client_id, so a frontend ID token could be replayed directly as an API access token (RFC 8725 §3.11).Why this direction
naruon's API credential is the OIDC access token, not the ID token:
frontend/src/app/auth/oidc/callback/route.ts:112andfrontend/src/app/auth/session/route.ts:288sendtoken_response.access_tokenas the bearer. There is noid_tokencredential path anywhere in the frontend.docker-compose.infra.yml,docker-compose.gateway.yml). Keycloak access tokens carry bodytyp: "Bearer"; ID tokens carrytyp: "ID".So the fix requires the token to be marked as an access token — Keycloak body
typ: "Bearer", or RFC 9068 headertyp: "at+jwt"— and rejects ID-token material and unmarked tokens.Change
_require_oidc_access_token(header, payload)to the OIDC decode path; reject anything not marked as an access token.typ: "ID", the unmarked PoC shape, a header-idvariant, and an accepted RFC 9068at+jwtaccess token.typ: "Bearer".developat HEAD5611121bcc9d21cf0cb551c18a8d6917c5c00f3d, resolving the prior merge conflict.http://127.0.0.1:3000/...targets reach Playwright; no Semgrep suppression remains.Current-head verification
uv run pytest tests/test_auth_real.py tests/test_repo_hygiene.py -q— 113 passed.uv run ruff check api/auth.py tests/test_auth_real.py tests/test_repo_hygiene.py— passed.uvx --from semgrep==1.169.0 semgrep --config p/javascript --error frontend/screenshot.cjs— 0 findings across 68 rules._decode_cached_oidc_session_payload -> _require_oidc_access_tokenplus rejection/acceptance tests inspected.Supersedes #1077
#1077 took the inverse approach (treat the ID token as the credential, reject
scope/scp). Run against a realistic Keycloak access token, #1077 rejects it and accepts the ID token. Keep #1077 closed/draft in favor of this PR.