fix(security): claim verified OIDC replay before token mint - #83
Draft
seonghobae wants to merge 8 commits into
Draft
fix(security): claim verified OIDC replay before token mint#83seonghobae wants to merge 8 commits into
seonghobae wants to merge 8 commits into
Conversation
📝 WalkthroughWalkthroughOIDC 교환 흐름이 서명·권한 검증 후 replay claim을 등록하고, 그 다음 GitHub 설치 토큰을 발급하도록 변경되었습니다. replay 충돌과 보호 기능 불가 상태는 각각 401과 503으로 처리됩니다. ChangesOIDC replay 보호
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Worker
participant ReplayGuard
participant GitHubAPI
Client->>Worker: POST /exchange
Worker->>ReplayGuard: claim verified jti and exp
ReplayGuard-->>Worker: claim accepted
Worker->>GitHubAPI: create installation token
GitHubAPI-->>Worker: token response
Worker-->>Client: 200 single-use response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
17 tasks
25 tasks
Contributor
Author
|
@coderabbitai review Please perform a substantive security review of exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='89f1d736c04a259363e566b2d269ad37fc6f8194'
echo '=== checked-out commit ==='
git rev-parse HEAD
git show -s --format='%H%n%P%n%s%n%D' HEAD
echo '=== expected commit available ==='
git cat-file -t "$expected"
echo '=== expected commit summary ==='
git show -s --format='%H%n%P%n%s%n%D' "$expected"
echo '=== changed files from first parent ==='
git diff-tree --no-commit-id --name-status -r "$expected"
echo '=== source outlines ==='
ast-grep outline src/index.ts --items all --view expanded || true
ast-grep outline src/worker.ts --items all --view expanded || true
ast-grep outline test/replay-before-token-mint.test.ts --items all --view expanded || true
echo '=== security-relevant source references ==='
rg -n -C 6 -e 'verifyGithubOidcJwt|createInstallationToken|replay|REPLAY|x-oidc-replay-protection|targetRepository|repository' src/index.ts src/worker.ts test/replay-before-token-mint.test.ts
echo '=== exact-head patch ==='
git diff "${expected}^" "$expected" -- src/index.ts src/worker.ts test/replay-before-token-mint.test.ts
echo '=== package scripts and dependency authority surface ==='
if [ -f package.json ]; then cat package.json; fi
if [ -f wrangler.toml ]; then cat wrangler.toml; fi
if [ -f wrangler.jsonc ]; then cat wrangler.jsonc; fi
echo '=== PR metadata and exact-head checks ==='
gh pr view 83 --repo ContextualWisdomLab/noema --json number,state,isDraft,headRefOid,baseRefName,baseRefOid,mergeStateStatus,statusCheckRollup,files \
--jq '{number,state,isDraft,headRefOid,baseRefName,baseRefOid,mergeStateStatus,checks:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}],files:[.files[]|{path,additions,deletions]}]}' || trueLength of output: 52580 🏁 Script executed: #!/bin/bash
set -euo pipefail
head='89f1d736c04a259363e566b2d269ad37fc6f8194'
base=$(git merge-base "$head" "origin/docs/architecture-trust-boundaries" 2>/dev/null || git merge-base "$head" "$head^")
printf '=== merge base ===\n%s\n' "$base"
echo '=== exact verification and core exchange path ==='
sed -n '352,410p' src/index.ts
sed -n '511,612p' src/index.ts
echo '=== complete production wrapper path ==='
sed -n '59,270p' src/worker.ts
sed -n '380,555p' src/worker.ts
echo '=== replay guard implementation ==='
replay_file=$(fd -t f '^oidc-replay\.ts$' src | head -n 1)
printf '%s\n' "$replay_file"
ast-grep outline "$replay_file" --items all --view expanded
cat -n "$replay_file"
echo '=== runtime entrypoint and bindings ==='
runtime_file=$(fd -t f '^runtime-entrypoint\.ts$' src | head -n 1)
printf '%s\n' "$runtime_file"
cat -n "$runtime_file"
echo '=== focused regression test bodies ==='
sed -n '163,255p' test/replay-before-token-mint.test.ts
echo '=== replay-related tests and direct-core tests ==='
rg -n -C 4 -e 'NOEMA_OIDC_REPLAY_GUARD|verified-before-mint|claimOidcTokenUsage|createInstallationToken|ERR_AUTH_REPLAY' test src -g '*.test.ts' -g '*.ts'
echo '=== cumulative branch diff from merge base ==='
git diff --stat "$base" "$head"
git diff --find-renames --find-copies "$base" "$head" -- src/index.ts src/worker.ts src/oidc-replay.ts src/runtime-entrypoint.ts wrangler.toml package.json test/replay-before-token-mint.test.ts docs/doctoring/replay-before-token-mint.md
echo '=== requested-commit documentation ==='
cat -n docs/doctoring/replay-before-token-mint.md
echo '=== exact-head PR and check metadata ==='
gh pr view 83 --repo ContextualWisdomLab/noema --json number,state,isDraft,headRefOid,baseRefName,baseRefOid,mergeStateStatus,statusCheckRollup,filesLength of output: 92060 🏁 Script executed: #!/bin/bash
set -euo pipefail
head='89f1d736c04a259363e566b2d269ad37fc6f8194'
pr_base='52c6f2bce0f89760367807df4c316f2137ad5a97'
echo '=== actual PR-base availability and ancestry ==='
git cat-file -t "$pr_base"
git merge-base --is-ancestor "$pr_base" "$head" && echo 'PR base is an ancestor of head'
git log --oneline --decorate --ancestry-path "${pr_base}..${head}"
echo '=== complete actual PR diff ==='
git diff --stat "$pr_base" "$head"
git diff --name-status "$pr_base" "$head"
git diff --find-renames --find-copies "$pr_base" "$head" -- \
src/index.ts src/worker.ts src/oidc-replay.ts src/runtime-entrypoint.ts \
wrangler.toml package.json \
'.github/**' test/replay-before-token-mint.test.ts docs/doctoring/replay-before-token-mint.md
echo '=== production wrapper dispatch, including route gate ==='
sed -n '356,555p' src/worker.ts
echo '=== all exported fetch entrypoints and exchange delegations ==='
rg -n -C 4 -P '(export default|async fetch\s*\(|baseWorker\.fetch\s*\(|handleExchange\s*\(|createInstallationToken\s*\()' src -g '*.ts'
echo '=== authority-related changes in actual PR ==='
git diff -U4 "$pr_base" "$head" -- \
wrangler.toml package.json \
':!src/index.ts' ':!src/worker.ts' ':!test/replay-before-token-mint.test.ts' ':!docs/doctoring/replay-before-token-mint.md'Length of output: 38305 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== entrypoint imports and /exchange delegation ==='
sed -n '1,55p' src/entrypoint.ts
sed -n '300,365p' src/entrypoint.ts
echo '=== worker import and runtime route-chain references ==='
rg -n -C 3 -P '(from "./worker"|worker\.fetch\s*\(|from "./entrypoint"|entrypoint\.fetch\s*\()' src -g '*.ts'Length of output: 6145
Diagnostic verdict: no security finding identified in the requested scope.
Limitations
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/doctoring/replay-before-token-mint.md (1)
1-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win운영 보안 문서를 한국어 중심으로 작성하세요.
이 문서는 replay 보호의 운영 및 보안 설계를 설명합니다. 본문과 제목을 한국어로 전환하세요. 코드, HTTP 헤더, 오류 코드, API 식별자, 인용 원문은 필요한 경우 영어로 유지하세요.
As per coding guidelines, operational and sales/acquisition documentation is largely Korean.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/doctoring/replay-before-token-mint.md` around lines 1 - 77, 이 문서의 제목과 설명 문단을 한국어 중심으로 번역해 운영·보안 설계 내용을 한국어로 제공하세요. 코드 블록, HTTP 헤더, 오류 코드, API 식별자, URL 및 필요한 인용 원문은 영어로 유지하고, 기술적 의미와 보안 순서·검증 조건은 변경하지 마세요.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/doctoring/replay-before-token-mint.md`:
- Around line 1-77: 이 문서의 제목과 설명 문단을 한국어 중심으로 번역해 운영·보안 설계 내용을 한국어로 제공하세요. 코드
블록, HTTP 헤더, 오류 코드, API 식별자, URL 및 필요한 인용 원문은 영어로 유지하고, 기술적 의미와 보안 순서·검증 조건은
변경하지 마세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 028bd60b-bad8-48bc-b52b-c1f1cd744422
📒 Files selected for processing (4)
docs/doctoring/replay-before-token-mint.mdsrc/index.tssrc/worker.tstest/replay-before-token-mint.test.ts
18 tasks
This was referenced Aug 9, 2026
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
1 participant
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.
Purpose
Test-first repair for #81: a replayed but otherwise valid GitHub Actions OIDC credential must be rejected by the distributed single-use claim after cryptographic/target authorization but before
createInstallationToken(), so duplicate requests cannot cross the privileged GitHub token-mint boundary. Unverifiedjtivalues must never consume replay state.Intended security ordering
Missing replay binding, replay conflict, unavailable replay state, missing bounded verified replay claims, or malformed proof fails closed before token mint.
Fresh stack identity
docs/architecture-trust-boundaries/ Draft fix(security): bind workflow source and document MSA boundaries #71.9df895084c451ac59b2299498e6197059c7203ab.9dfbe1007c43e5c3ca1381ad2a57d748776f0c88.52c6f2bce0f89760367807df4c316f2137ad5a97.main; fix(security): claim verified OIDC replay before token mint #83 therefore remains Draft and write-deferred.src/index.ts,src/worker.ts,test/replay-before-token-mint.test.ts,test/distributed-rate-limit.test.ts,test/worker-defensive-replay.test.ts, anddocs/doctoring/replay-before-token-mint.md.Protected
mainis nowc85d710804139c0697d7ef8fa47d02b1389e6d84; #76 is integrated and the old repository-wide nanoid audit failure is historical. That does not make this stale feature-base head current or transfer its old checks.Test-first lineage
ed1e7c529712028c4f6e5c5bf928fb07bba48a18: RED valid-signed-replay regression requiring 401ERR_AUTH_REPLAYand zero installation-token POSTs.12c95f033437cfc7efbdfd61036775921267ebf2: exposes verifiedjti/exponly after target authorization and beforecreateInstallationToken().1e893dda8ba063ffaaa694521f4cfac80d1f86dc: wrapper requires distributed replay binding and consumes pre-mint proof.5acd2f0f3c69a9e80f3878ab9359dd1829c061b1: first-use ordering requiresreplay_claimbeforetoken_mint.9dfbe1007c43e5c3ca1381ad2a57d748776f0c88: current stale head; defensive replay-unavailable/unexpected-guard coverage.Historical exact-head evidence
For
9dfbe1007c43e5c3ca1381ad2a57d748776f0c88, application CI31316088354reached 679 tests and configured statements/branches/functions/lines 100% before the then-inherited nanoid audit; reviewer-ci31316088356was terminal success; central Security Scan was absent because the PR targets a feature base and therefore remained non-passingdefer_until_trigger. Current inline review threads were 0 at the last exact read.The old audit failure no longer describes protected main, but all evidence remains bound to this stale head and does not transfer after convergence.
Current dependency / ownership rule
Do not refresh or patch #83 while #71 is a moving/diverged canonical shared-source owner. The safe path is:
No repair workflow, self-modifying Action, broader App permission, audit waiver, protection bypass, synthetic approval, merge/release/deployment authority is introduced.