Skip to content

fix(security): bind workflow source and document MSA boundaries - #71

Open
seonghobae wants to merge 107 commits into
mainfrom
docs/architecture-trust-boundaries
Open

fix(security): bind workflow source and document MSA boundaries#71
seonghobae wants to merge 107 commits into
mainfrom
docs/architecture-trust-boundaries

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Establishes Noema's canonical product/technical/architecture documentation spine while hardening credential exchange so trusted GitHub Actions workflow identity is bound to an exact workflow ref plus immutable workflow-file commit SHA. The same canonical graph defines fail-closed licensing/IP-transfer evidence and now records the private-target reviewer authentication decision without promoting the unmerged #85 implementation to protected-main fact.

This PR owns the repository-wide product/runtime/trust/evidence documentation model. Focused implementation work remains separately owned by #80 (scheduler/publisher), #83 (replay-before-token-mint), and #85 (private-target review authentication).

Exact source identity

Freshly revalidated:

  • Head branch: docs/architecture-trust-boundaries.
  • Exact current head: 480ea86b1dcf93508aaf1ae759b8f8f19b497c96.
  • Base branch: main.
  • Independently resolved/live base tip: 9adb784902cae19a150146a8fadd6a0491f67814.
  • Direct-main architecture/security PR; no stack predecessor.
  • Mergeable according to GitHub's content graph, but not merge-authorized under Noema governance.

Runtime trust implementation

  • root ARCHITECTURE.md documents runtime-entrypoint → entrypoint → worker → core exchange, /health, /ready, /exchange, Durable Object state, evidence planes, and standalone/MSA composition;
  • ALLOWED_WORKFLOW_SHA is validated canonically by readiness and credential-bearing trust logic;
  • caller workflow_ref/workflow_sha and reusable job_workflow_ref/job_workflow_sha families are validated independently; a complete reusable pair is the called-workflow authority even when complete caller metadata is also present;
  • missing, malformed, orphaned or non-canonical claim members, parser disagreement, oversized/malformed bearer data and ref/SHA mismatch fail closed before credential-bearing delegation;
  • readiness-binding failures remain distinct from request-token authorization failures;
  • Durable Object alarm semantics document bounded automatic retry and explicit rescheduling for recovery beyond the platform retry budget.

Canonical documentation graph

This branch provides and indexes:

  • docs/PRD.md — users, buyer/operator problems, modes, standalone/MSA outcomes, FR/NFRs, degraded behavior, acceptance, non-goals;
  • docs/TRD.md — exact revision/live-base evidence, evidence-class separation, workflow identity, pagination, reviewer eligibility, writer lease, RCA/feasibility, work-conserving continuation, OpenCode/NVIDIA NIM, packaging/release/provenance;
  • root ARCHITECTURE.md;
  • docs/adr/README.md plus ADR-0001..0010;
  • docs/UML.md with component, sequence, state, authority-flow and deployment/control-plane views;
  • docs/ERD.md separating actual Durable Object persistence from conceptual GitHub/evidence entities;
  • docs/TRACEABILITY.md mapping requirements/ADRs/standards to source, tests, issues and evidence;
  • docs/TEST_STRATEGY.md, docs/OPERABILITY.md;
  • docs/LICENSING_AND_IP_TRANSFER.md — owner/legal authority separation, package metadata alignment, SPDX/custom/UNLICENSED semantics, exact-release SBOM + dependency-license + NOTICE obligations, contributor/assignment provenance, and planned transfer-evidence binding;
  • runtime and automation threat models;
  • docs/DOCUMENTATION_GAP_AUDIT.md with sufficiency verdict, implemented-vs-proposed separation, residual owners, and protected-main acceptance criteria;
  • docs/README.md, CLAUDE.md, API/runbook/doctoring and CHANGELOG.md integration.

test/documentation-architecture-contract.test.ts makes the required graph, status/authority separation, UML/ERD semantics, no-early-stop contract, licensing/IP-transfer boundary, private-target review-auth ADR, and CHANGELOG alignment executable.

Work-conserving deliverable handoff

FR-019 and ADR-0002 require intermediate artifacts to continue to the next safe authority or acceptance boundary:

prompt update → repository-consumed policy and executable contract
RCA → feasible action
design → implementation
test → production code
documentation assessment → canonical repository files
local changes → intentional commit → pull request
pull request → exact-head checks → review remediation → protected merge
protected merge → protected-main operational acceptance → queue top

A blocked handoff defers only that lane. Documentation repair is intermediate; it is not invocation or product completion. Termination requires a double fresh exit sweep or genuine practical run-budget exhaustion, never a status report.

Licensing/IP transfer boundary

A protected-main audit found no root LICENSE file and current package.json is "private": true without a license field. This is an unresolved rights/transfer decision, not evidence for any outbound license.

The branch requires these authorities to remain separate:

  • automation may inventory, hash, compare, and fail closed on missing or contradictory rights evidence;
  • automation must not choose an outbound license or fabricate contributor/assignment evidence;
  • an authorized owner/legal decision must precede any approved license posture;
  • package metadata must match that decision using an SPDX expression, SEE LICENSE IN <filename>, or UNLICENSED where appropriate;
  • distributable/acquirable exact releases require SBOM-bound dependency-license and NOTICE/attribution evidence plus contributor/IP ownership/assignment provenance;
  • transfer-evidence.json presence alone is not proof until its licensing/IP fields and immutable evidence references are actually validated.

Issue #5 remains the external licensing/transfer authority boundary. Repository-owned consistency/inventory controls remain executable follow-on work, but this PR does not invent legal clearance.

Private-target reviewer decision

ADR-0010 records the authority boundary exposed by #85:

syntactic target identifier validation
→ repository-scoped Noema App read token
→ authenticated live target PR binding
→ exact dispatched head/base/repository validation
→ trusted reviewer/evidence flow

The automatically generated workflow-repository GITHUB_TOKEN is not treated as cross-repository private-target authority. The ADR records least-privilege App-token reuse, rejection of PAT/broad-token/publication workarounds, and protected-main operational acceptance requirements. #85 remains the focused implementation PR; its unmerged behavior is In review, not protected-main architecture fact.

Documentation sufficiency verdict

Before this branch, strong API/runtime/runbook/release/acquisition material existed but product, authority and architecture decisions were scattered across PR bodies, issues, workflows and conversation history. The current branch is substantially sufficient as a proposed canonical baseline. It is not protected-main accepted, release-complete, deployment-proven, legally cleared, or acquisition-ready. DOCUMENTATION_GAP_AUDIT.md keeps those distinctions and residual owners explicit.

Current continuation lineage

The earlier architecture/security and documentation-contract lineage remains in commit history. Since the previous documented head 319fdaadf4af3da130ada0fe284159e95720da75, the current head is five commits ahead and changes only the documentation/control-contract surface:

  • docs/DOCUMENTATION_GAP_AUDIT.md: refreshes status/owner classification against active implementation PRs and external control-plane evidence;
  • docs/TRACEABILITY.md: extends active-work/authority mapping;
  • docs/adr/0010-private-target-review-auth.md: adds the private-target reviewer auth decision;
  • docs/adr/README.md: indexes ADR-0010;
  • test/documentation-architecture-contract.test.ts: makes the new ADR/status separation executable.

The exact comparison from 319f... to current head changes only those five files; no runtime source is changed by that final five-commit continuation.

Exact-head verification and RCA

For exact head 480ea86b1dcf93508aaf1ae759b8f8f19b497c96:

  • ci run 31320691901: terminal failure. The runner executed the current exact/synthetic integration checkout contract, typecheck succeeded, and the application suite completed 678/678 tests with configured production statement, branch, function and line coverage all 100%. The only failing acceptance boundary is repository-wide npm audit --audit-level=high on inherited nanoid@3.3.16 (GHSA-2v37-7h3g-55p8). The root remediation is isolated in PR fix(security): update transitive nanoid for CVE-2026-67213 #76; duplicating the lockfile change here is rejected as a wrong-layer fix.
  • reviewer-ci run 31320691937: terminal success.
  • Central Security Scan run 31320691919: terminal success.
  • All currently visible inline review threads: resolved; unresolved count 0.
  • Formal review submissions are COMMENTED/historical evidence only; no eligible independent non-author APPROVED review exists.

Remediation feasibility

Active proposed and external work

Unmerged implementation is not promoted to protected-main fact:

External/control-plane evidence remains external:

Required acceptance before merge

  1. fix(security): update transitive nanoid for CVE-2026-67213 #76 integrates under actual live governance and removes the inherited dependency-audit blocker.
  2. Refetch live main; if it moved, refresh this branch without losing its unique architecture/security/docs work.
  3. Require exact current-head application CI, reviewer CI and Security Scan terminal success; predecessor/synthetic-only evidence cannot substitute where exact-head evidence is required.
  4. Re-review current exact head and keep zero valid unresolved findings/threads.
  5. Satisfy enforceable main ruleset requirements and qualifying independent non-author approval.
  6. After protected integration, run protected-main operational acceptance for workflow-ref/SHA trust, readiness failure separation, and any integrated private-target/replay behavior that this architecture claims.

Release decision

No version bump, release, deployment, legal-clearance, or acquisition-ready claim is made. Documentation completion is an intermediate repository improvement, not a release, transfer, or acquisition gate substitute.

Related: #5, #27, #29, #30, #40, #65, #66, #67, #72, #73, #75, #76, #77, #78, #80, #81, #82, #83, #84, #85

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Noema의 런타임 구조와 신뢰 경계를 문서화했습니다. Worker는 workflow ref와 immutable SHA pair를 검증합니다. readiness, API 계약, 운영 문서와 회귀 테스트를 갱신했습니다.

Changes

아키텍처 및 workflow 신뢰

Layer / File(s) Summary
런타임 credential exchange
src/worker.ts, src/runtime-readiness.ts, wrangler.toml, docs/api-spec.md, test/*
Worker가 reusable 및 일반 workflow의 ref·SHA pair를 검증합니다. 설정 오류는 503으로 처리하고 claim 불일치는 403으로 처리합니다.
보안 경계와 분산 상태
ARCHITECTURE.md, docs/doctoring/architecture-trust-boundaries.md, docs/runbook.md
Cloudflare binding, outbound 요청, Durable Object 상태, alarm 재시도와 exact-head 규칙을 문서화했습니다.
서비스 조합과 권한 plane
ARCHITECTURE.md, docs/doctoring/architecture-trust-boundaries.md, docs/onboarding.md, docs/runbook.md
중앙 workflow, 리뷰 서비스와 Noema의 역할을 분리했습니다. 독립 배포, versioned contract와 authority plane을 정의했습니다.
운영 문서와 회귀 검증
CLAUDE.md, docs/api-spec.md, README.md, CHANGELOG.md, test/*
/health, /ready, /exchange, workflow SHA 설정과 운영 검증 절차를 문서화했습니다. readiness, 문서 계약과 workflow trust 회귀 테스트를 추가·갱신했습니다.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant CloudflareWorker
  participant NoemaOidcReplayGuard
  participant GitHubApp
  GitHubActions->>CloudflareWorker: /exchange 요청
  CloudflareWorker->>CloudflareWorker: workflow ref·SHA claims 검증
  CloudflareWorker->>NoemaOidcReplayGuard: jti replay 검증
  CloudflareWorker->>GitHubApp: installation token 교환
  GitHubApp-->>CloudflareWorker: 저장소 범위 단기 토큰
  CloudflareWorker-->>GitHubActions: exchange 응답
Loading

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 immutable workflow source 검증 강화와 MSA 경계 문서화를 모두 간결하게 요약하며 변경 내용과 직접 관련됩니다.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/architecture-trust-boundaries

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
docs/doctoring/architecture-trust-boundaries.md (1)

70-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Durable Object alarm의 제한된 자동 재시도 semantics를 기록하세요.

현재 문서는 alarm이 at-least-once로 실행되고 실패 시 자동 재시도된다고 설명합니다. Cloudflare 공식 문서는 자동 재시도가 exponential backoff와 최대 6회로 제한되며, 무기한 복구가 필요하면 handler가 새 alarm을 명시적으로 예약해야 한다고 설명합니다. 이 제한과 reschedule 요구사항을 추가하세요. (developers.cloudflare.com)

🤖 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/architecture-trust-boundaries.md` around lines 70 - 78, Update
the Durable Object alarm semantics section to state that automatic retries use
exponential backoff and are limited to a maximum of six attempts. Also document
that handlers requiring recovery beyond those retries must explicitly schedule a
new alarm, while preserving the existing at-least-once and stale-state
validation guidance.
test/architecture-documentation.test.ts (1)

5-35: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

문서 회귀 검사를 실제 runtime contract 검사로 강화하세요.

현재 테스트는 required 문자열과 두 개의 오래된 문구만 검사합니다. wrangler.tomlmain, src/entrypoint.ts·src/worker.ts·src/index.ts의 layer ownership, Durable Object binding/class, route method를 비교하지 않습니다. 따라서 runtime 구조가 바뀌어도 일부 키워드가 남아 있으면 테스트가 통과합니다. 문서와 실제 설정·소스의 핵심 관계를 직접 assertion으로 추가하세요.

🤖 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 `@test/architecture-documentation.test.ts` around lines 5 - 35, Strengthen the
architecture documentation tests beyond keyword checks by asserting the runtime
contract against actual configuration and source ownership. In the existing
tests, read wrangler.toml and the relevant src/entrypoint.ts, src/worker.ts, and
src/index.ts files, then validate the configured main entrypoint, layer
ownership, Durable Object binding/class, and route HTTP methods against the
documented architecture. Keep the existing regression assertions while making
these relationships fail when runtime structure changes.
🤖 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.

Inline comments:
In `@ARCHITECTURE.md`:
- Around line 11-14: Update the exact workflow trust logic in src/worker.ts to
require immutable SHA-based identity: validate workflow_sha and job_workflow_sha
in addition to the full workflow_ref, rather than trusting mutable branch or tag
refs alone. Update ARCHITECTURE.md lines 11-14 and 24-33 and CLAUDE.md lines 9
and 31-36 so their trust descriptions accurately state the SHA validation and
immutable source binding performed by src/worker.ts.

In `@CLAUDE.md`:
- Line 27: README의 운영 도구 설명과 실제 문서 내용을 정합화하세요. 각 명령(kpi:compute, smoke:check,
production:preflight, readiness:audit, acquisition:manifest/acquisition:audit,
security:evidence)의 필수 NOEMA_* 환경 변수를 README에 명령별로 추가하고, 기존 Worker 및 KPI 수집 예시와
구분해 문서화하세요. 실제 요구 변수를 확인할 수 없다면 해당 변수를 문서화한다고 주장하지 않도록 CLAUDE.md의 문장을 범위에 맞게
축소하세요.

---

Nitpick comments:
In `@docs/doctoring/architecture-trust-boundaries.md`:
- Around line 70-78: Update the Durable Object alarm semantics section to state
that automatic retries use exponential backoff and are limited to a maximum of
six attempts. Also document that handlers requiring recovery beyond those
retries must explicitly schedule a new alarm, while preserving the existing
at-least-once and stale-state validation guidance.

In `@test/architecture-documentation.test.ts`:
- Around line 5-35: Strengthen the architecture documentation tests beyond
keyword checks by asserting the runtime contract against actual configuration
and source ownership. In the existing tests, read wrangler.toml and the relevant
src/entrypoint.ts, src/worker.ts, and src/index.ts files, then validate the
configured main entrypoint, layer ownership, Durable Object binding/class, and
route HTTP methods against the documented architecture. Keep the existing
regression assertions while making these relationships fail when runtime
structure changes.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4bd0e593-d123-4411-ad24-d5e917057839

📥 Commits

Reviewing files that changed from the base of the PR and between 9adb784 and 06ae6aa.

📒 Files selected for processing (6)
  • ARCHITECTURE.md
  • CHANGELOG.md
  • CLAUDE.md
  • README.md
  • docs/doctoring/architecture-trust-boundaries.md
  • test/architecture-documentation.test.ts

Comment thread ARCHITECTURE.md
Comment thread CLAUDE.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@test/workflow-sha-trust.test.ts`:
- Around line 122-132: Extend the workflow trust tests around
expectWorkflowBlock to cover runtimeEnv without ALLOWED_WORKFLOW_SHA, asserting
it resolves with status 503 and message "Workflow trust configuration
unavailable". Keep the existing malformed-SHA case unchanged and exercise the
undefined binding branch explicitly.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cee8500d-f924-4bb0-9314-7c44ed164834

📥 Commits

Reviewing files that changed from the base of the PR and between 06ae6aa and fadc728.

📒 Files selected for processing (2)
  • src/worker.ts
  • test/workflow-sha-trust.test.ts

Comment thread test/workflow-sha-trust.test.ts
@seonghobae seonghobae changed the title docs(architecture): define current trust and MSA boundaries fix(security): bind workflow source and document MSA boundaries Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact head 52c6f2bce0f89760367807df4c316f2137ad5a97 after the documentation/API-contract corrections. Focus on whether the PRD/TRD/ADR/UML/ERD/traceability/operability/threat-model graph accurately distinguishes Implemented vs Proposed vs External evidence, whether the malformed-Bearer 400 vs missing/cryptographically-invalid 401 contract now matches runtime behavior, and whether any new architecture statement overstates the replay-before-token-mint fix that is still only Draft PR #83. Treat this review as diagnostic model evidence only, not formal approval or merge authority.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='52c6f2bce0f89760367807df4c316f2137ad5a97'
actual="$(git rev-parse HEAD)"
printf 'expected=%s\nactual=%s\n' "$expected" "$actual"
if [ "$actual" != "$expected" ]; then
  printf 'STALE_OR_MISMATCHED_HEAD\n' >&2
  exit 42
fi

printf '\n== exact commit and files changed by commit ==\n'
git show -s --format='commit=%H%nparents=%P%nsubject=%s%n%n%b' "$actual"
git diff-tree --no-commit-id --name-status -r "$actual"

printf '\n== PR changes since prior reviewed head ==\n'
git diff --name-status 'e3c16df74508c082d00553011a5007ac3a7adfad' "$actual" || true

printf '\n== canonical documentation inventory ==\n'
fd -t f -E node_modules -E dist . docs | rg -i \
  '(^|/)(PRD|TRD|UML|ERD|TRACEABILITY|TEST_STRATEGY|OPERABILITY|THREAT|ARCHITECTURE|DOCUMENTATION_GAP_AUDIT)\.md$|/adr/.*\.md$' || true

printf '\n== entrypoint and exchange maps ==\n'
ast-grep outline src/entrypoint.ts src/runtime-entrypoint.ts src/worker.ts src/index.ts --items all --view expanded || true

printf '\n== runtime Bearer parsing, routing, and exchange delegation ==\n'
rg -n -i -C 5 \
  '(authorization|bearer|ERR_TOKEN_MALFORMED|token.*malformed|401|400|/exchange|baseWorker|base\.fetch|createInstallationToken|claimOidcTokenUsage)' \
  src/entrypoint.ts src/runtime-entrypoint.ts src/worker.ts src/index.ts || true

printf '\n== API and stability contracts ==\n'
for f in docs/api-stability-contract.md docs/api-spec.md; do
  if [ -f "$f" ]; then
    printf '\n--- %s ---\n' "$f"
    rg -n -i -C 4 \
      '(bearer|malformed|oversized|missing|cryptograph|401|400|ERR_TOKEN_MALFORMED|/exchange)' "$f"
  fi
done

printf '\n== documentation classifications and replay claims ==\n'
rg -n -i -C 3 \
  '(implemented|proposed|planned|external.?evidence|authority|replay|side.effect|token.mint|createInstallationToken|`#81`|`#83`|draft)' \
  ARCHITECTURE.md docs README.md CLAUDE.md CHANGELOG.md 2>/dev/null || true

printf '\n== documentation regression tests ==\n'
rg -n -i -C 3 \
  '(api-stability-contract|ERR_TOKEN_MALFORMED|malformed.*bearer|`#81`|`#83`|replay|side.effect|implemented|proposed|external)' \
  test 2>/dev/null || true

printf '\n== Draft PR 83 metadata ==\n'
gh pr view 83 --repo ContextualWisdomLab/noema \
  --json number,state,isDraft,title,headRefName,baseRefName,url,body

Length of output: 195428


🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='52c6f2bce0f89760367807df4c316f2137ad5a97'
test "$(git rev-parse HEAD)" = "$expected"

printf '%s\n' '== exact commit summary and patch =='
git show --format=fuller --stat --find-renames "$expected"
git show --format= --find-renames "$expected" -- \
  docs/api-stability-contract.md \
  docs/DOCUMENTATION_GAP_AUDIT.md \
  docs/UML.md \
  docs/PRD.md docs/TRD.md docs/TRACEABILITY.md docs/OPERABILITY.md \
  docs/threat-model.md docs/automation-threat-model.md \
  test/architecture-documentation.test.ts \
  test/documentation-architecture-contract.test.ts

printf '%s\n' '== deployed malformed-Bearer handling =='
nl -ba src/entrypoint.ts | sed -n '185,245p'
nl -ba src/entrypoint.ts | sed -n '300,335p'
nl -ba docs/api-stability-contract.md | sed -n '20,48p'
nl -ba docs/api-spec.md | sed -n '125,165p'
nl -ba test/architecture-documentation.test.ts | sed -n '125,145p'
nl -ba test/github-api-egress.test.ts | sed -n '70,125p'

printf '%s\n' '== actual replay order =='
nl -ba src/worker.ts | sed -n '385,480p'
nl -ba src/index.ts | sed -n '535,650p'
nl -ba test/oidc-replay.test.ts | sed -n '220,250p'

printf '%s\n' '== canonical replay statements =='
for f in ARCHITECTURE.md docs/UML.md docs/DOCUMENTATION_GAP_AUDIT.md \
  docs/oidc-replay-protection.md docs/PRD.md docs/TRD.md \
  docs/TRACEABILITY.md docs/OPERABILITY.md docs/threat-model.md \
  docs/automation-threat-model.md; do
  [ -f "$f" ] || continue
  printf '\n--- %s ---\n' "$f"
  rg -n -i -C 3 \
    '(`#81`|`#83`|replay.{0,100}(claim|mint|side.effect|before|after)|createInstallationToken|post.mint|pre.mint|zero.{0,30}(side.effect|mint))' \
    "$f" || true
done

printf '%s\n' '== documentation tests that protect replay ordering/caveat =='
rg -n -i -C 4 \
  '(DOCUMENTATION_GAP_AUDIT|oidc-replay-protection|`#81`|`#83`|createInstallationToken|post.exchange|post-mint|replay.{0,80}(before|after|side.effect))' \
  test/architecture-documentation.test.ts test/documentation-architecture-contract.test.ts test || true

Length of output: 3639


🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='52c6f2bce0f89760367807df4c316f2137ad5a97'
test "$(git rev-parse HEAD)" = "$expected"

lines() {
  local file="$1" start="$2" end="$3"
  printf '\n--- %s: lines %s-%s ---\n' "$file" "$start" "$end"
  awk -v start="$start" -v end="$end" \
    'NR >= start && NR <= end { printf "%d: %s\n", NR, $0 }' "$file"
}

printf '%s\n' '== deployed malformed-Bearer handling and contract =='
lines src/entrypoint.ts 185 245
lines src/entrypoint.ts 300 335
lines docs/api-stability-contract.md 20 48
lines docs/api-spec.md 125 165
lines test/architecture-documentation.test.ts 125 145
lines test/github-api-egress.test.ts 70 125

printf '%s\n' '== actual replay ordering =='
lines src/worker.ts 385 480
lines src/index.ts 535 650
lines test/oidc-replay.test.ts 220 250

printf '%s\n' '== canonical replay documentation =='
for f in ARCHITECTURE.md docs/UML.md docs/DOCUMENTATION_GAP_AUDIT.md \
  docs/oidc-replay-protection.md docs/PRD.md docs/TRD.md \
  docs/TRACEABILITY.md docs/OPERABILITY.md docs/threat-model.md \
  docs/automation-threat-model.md; do
  [ -f "$f" ] || continue
  printf '\n--- %s ---\n' "$f"
  rg -n -i -C 3 \
    '(`#81`|`#83`|replay.{0,100}(claim|mint|side.effect|before|after)|createInstallationToken|post.mint|pre.mint|zero.{0,30}(side.effect|mint))' \
    "$f" || true
done

printf '%s\n' '== documentation test coverage for replay caveat =='
rg -n -i -C 4 \
  '(DOCUMENTATION_GAP_AUDIT|oidc-replay-protection|`#81`|`#83`|createInstallationToken|post.exchange|post-mint|replay.{0,80}(before|after|side.effect))' \
  test || true

Length of output: 35481


@seonghobae I reviewed exact head 52c6f2bce0f89760367807df4c316f2137ad5a97. The checked-out HEAD matched the requested commit.

I found two actionable documentation issues.

  • docs/oidc-replay-protection.md Lines 81-82 instruct operators to verify that token reuse returns 401 “without a second token.” This statement can mean that GitHub does not mint a second installation token. That is not true for the current implementation. src/worker.ts calls baseWorker.fetch() before claimOidcTokenUsage(), and src/index.ts mints the installation token in the base exchange. The same document correctly states on Lines 37-41 that a replay can leave an undelivered upstream token. Change the deployment instruction to say that reuse returns 401 without delivering a second token. State that the pre-mint guarantee is only proposed in Draft PR #83.

  • The canonical replay caveat has no focused documentation regression test. test/oidc-replay.test.ts correctly locks the current source order: replay consumption occurs after successful base exchange. However, test/documentation-architecture-contract.test.ts only checks that docs/DOCUMENTATION_GAP_AUDIT.md exists. It does not require the issue #81 statement, the current post-mint order, or the prohibition on zero-side-effect claims. Add a focused assertion for these facts. This will prevent a future documentation edit from claiming the Draft PR #83 behavior before protected integration and verification.

The malformed-Bearer contract is now correct. src/entrypoint.ts returns 400 ERR_TOKEN_MALFORMED for malformed or oversized envelopes. docs/api-stability-contract.md now reserves 401 for missing or cryptographically invalid tokens. The regression test covers this distinction.

The PRD, TRD, ADR, UML, ERD, traceability, operability, and threat-model documents generally separate Implemented, Proposed, and External evidence. They also preserve the evidence-versus-authority boundary.

This review is diagnostic evidence only. It is not approval, merge authority, release acceptance, or deployment proof.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please re-review exact head c55a01dba00096b334035e65c377aaf33bd430f2. The predecessor-head finding about protecting the #81 replay-side-effect caveat is addressed by a focused executable documentation contract that requires the current post-token-mint limitation, #81 ownership, and the required post-verification/pre-createInstallationToken() repair boundary to remain explicit until integration. Also verify the earlier malformed-Bearer 400/401 documentation correction and the canonical PRD/TRD/ADR/UML/ERD/traceability/operability status separation. Treat your verdict as diagnostic model evidence only, not formal GitHub approval or merge authority.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

expected='c55a01dba00096b334035e65c377aaf33bd430f2'
actual="$(git rev-parse HEAD)"
printf 'expected=%s\nactual=%s\n' "$expected" "$actual"
if [ "$actual" != "$expected" ]; then
  printf 'STALE_OR_MISMATCHED_HEAD\n' >&2
  exit 42
fi

printf '\n== exact commit ==\n'
git show -s --format='commit=%H%nparents=%P%nsubject=%s%n%n%b' "$actual"

printf '\n== changes since predecessor reviewed head ==\n'
git diff --name-status '52c6f2bce0f89760367807df4c316f2137ad5a97' "$actual"

printf '\n== focused diff ==\n'
git diff --find-renames '52c6f2bce0f89760367807df4c316f2137ad5a97' "$actual" -- \
  docs test ARCHITECTURE.md CLAUDE.md README.md CHANGELOG.md || true

printf '\n== relevant source structure ==\n'
ast-grep outline src/worker.ts src/index.ts src/entrypoint.ts --items all --view expanded

printf '\n== replay order in source ==\n'
rg -n -C 6 '(baseWorker\.fetch|createInstallationToken|claimOidcTokenUsage|replay)' \
  src/worker.ts src/index.ts

printf '\n== focused documentation-contract tests ==\n'
rg -n -C 8 '(`#81`|`#83`|createInstallationToken|post-token|post.mint|pre-.*mint|replay.{0,100}(side.effect|claim|repair))' \
  test --glob '*.ts'

printf '\n== replay documentation ==\n'
rg -n -i -C 5 '(`#81`|`#83`|createInstallationToken|post.token.mint|post.mint|pre.*createInstallationToken|replay.{0,100}(side.effect|claim|repair)|zero.{0,40}(side.effect|mint))' \
  ARCHITECTURE.md docs --glob '*.md'

printf '\n== deployed Bearer envelope handling ==\n'
sed -n '180,245p' src/entrypoint.ts
sed -n '300,340p' src/entrypoint.ts

printf '\n== API Bearer contracts ==\n'
for f in docs/api-stability-contract.md docs/api-spec.md; do
  printf '\n--- %s ---\n' "$f"
  rg -n -i -C 5 '(bearer|malformed|oversized|ERR_TOKEN_MALFORMED|cryptograph|missing|401|400)' "$f"
done

printf '\n== canonical status separation ==\n'
for f in docs/PRD.md docs/TRD.md docs/UML.md docs/ERD.md docs/TRACEABILITY.md \
         docs/OPERABILITY.md docs/threat-model.md docs/automation-threat-model.md \
         docs/adr/*.md; do
  [ -f "$f" ] || continue
  printf '\n--- %s ---\n' "$f"
  rg -n -i -C 2 '(implemented|proposed|planned|external.?evidence|draft|authority)' "$f" || true
done

Length of output: 79707


@seonghobae I reviewed exact head c55a01dba00096b334035e65c377aaf33bd430f2. The checked-out HEAD matched the requested commit.

I found one actionable issue.

  • docs/oidc-replay-protection.md Line 82 still states that immediate token reuse returns 401 “without a second token.” This can mean that GitHub does not create a second installation token. The current execution order does not provide that guarantee. src/worker.ts calls baseWorker.fetch() before claimOidcTokenUsage(), and src/index.ts calls createInstallationToken() during that base exchange. Change the statement to say that reuse returns 401 without delivering a second token. State that preventing the pre-delivery GitHub token-mint side effect requires the #81 repair after cryptographic OIDC and target authorization, but before createInstallationToken().

The new executable documentation contract addresses the prior documentation-test finding. It requires the current post-token-mint limitation, Issue #81``, and the required repair boundary to remain explicit.

The malformed-Bearer contract is correct. The runtime returns 400 ERR_TOKEN_MALFORMED for malformed or oversized Bearer envelopes. The API stability contract reserves 401 for missing or cryptographically invalid Bearer tokens.

The PRD, TRD, ADR, UML, ERD, traceability, and operability documents maintain the Implemented, Proposed or Planned, and External evidence separation.

This review is diagnostic model evidence only. It is not GitHub approval, merge authority, release acceptance, or deployment proof.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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