fix(security): bind workflow source and document MSA boundaries - #71
fix(security): bind workflow source and document MSA boundaries#71seonghobae wants to merge 145 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughNoema의 런타임 구조와 신뢰 경계를 문서화했습니다. Worker는 workflow ref와 immutable SHA pair를 검증합니다. readiness, API 계약, 운영 문서와 회귀 테스트를 갱신했습니다. Changes아키텍처 및 workflow 신뢰
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 응답
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/doctoring/architecture-trust-boundaries.md (1)
70-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDurable 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.toml의main,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
📒 Files selected for processing (6)
ARCHITECTURE.mdCHANGELOG.mdCLAUDE.mdREADME.mddocs/doctoring/architecture-trust-boundaries.mdtest/architecture-documentation.test.ts
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
src/worker.tstest/workflow-sha-trust.test.ts
seonghobae
changed the title
1 participant
Purpose
Maintain the single canonical Noema documentation/architecture graph: PRD, TRD, root Architecture, ADRs, UML, conceptual/logical ERD, Test Strategy, Operability, licensing/IP-transfer, threat models and requirement→source/test/evidence traceability. Active-PR behavior remains
Proposed/In review, never protected-main truth.Fresh source identity
main:c85d710804139c0697d7ef8fa47d02b1389e6d84(fix(security): update transitive nanoid for CVE-2026-67213 #76 integrated;nanoid@3.3.17is protected truth);docs/architecture-trust-boundaries;9df895084c451ac59b2299498e6197059c7203ab;main: diverged, 141 commits ahead / 1 behind, merge base9adb784902cae19a150146a8fadd6a0491f67814;Documentation sufficiency verdict
A physical relational ERD would be misleading today: current runtime persistence is Durable Object state while GitHub/review/release/acquisition entities are conceptual/external evidence entities until Noema actually owns a durable evidence database.
Current successor ownership
The canonical docs were refreshed to current live ownership and historical-lineage truth:
Latest documentation changes on this branch
The active-work documentation contract now requires current clean successors #90–#95 plus #97, and the canonical Traceability/Gap Audit now distinguish:
mainfrom historical pre-fix(security): update transitive nanoid for CVE-2026-67213 #76 base evidence.Latest commits in this continuation:
02e2c4bf92eb0e0b7b0f42f8e504d908bb798bb2— test contract requiring current successor ownership;79e38017757d904d87c5ad87e63bcae19f8ef391— current disclosure/scheduler evidence traceability;9df895084c451ac59b2299498e6197059c7203ab— documentation gap audit current-owner refresh.Exact-head evidence boundary
No fresh application/reviewer/Security Scan workflow runs are currently attached to head
9df895084c451ac59b2299498e6197059c7203abthrough the available PR-run evidence endpoint. ACodeRabbitcommit status is present, but status-only evidence is not CI, formal review or merge authority. Therefore this head is not considered verified or merge-ready.Historical #71 checks from earlier heads and the pre-#76 synthetic integration are diagnostic only and do not transfer.
Protected-main guidance drift
Protected
mainAGENTS.mdstill teaches stale central Security Scan semantics (including stacked PRsand CRITICAL/HIGH-only Trivy language). PR #90 is the clean correction owner: protected-base eligibility, feature-base absence as non-passingdefer_until_trigger, and MEDIUM/HIGH/CRITICAL. Until protected integration, live central evidence overrides stale prose.Licensing/IP boundary
Public visibility, package metadata, SBOM/scanner output or OCI annotations do not create outbound legal authority. Issue #5 owns owner/legal and contributor/IP-transfer evidence; PR #69 owns technical exact-release consistency (
artifact_rights_metadata, authenticated bytes, duplicate decoded-key and malformed UTF-8 refusal). Automation does not choose a license.Safe continuation / merge boundary
This PR remains non-mergeable on a diverged pre-#76 lineage. Safe continuation is to preserve this canonical graph while avoiding destructive history rewriting, then perform a controlled protected-main convergence that preserves #76 and current successor contracts, reacquire exact-head application/reviewer/eligible Security Scan evidence, resolve every valid current finding, satisfy actual live governance and qualifying formal review where required, and only then merge. Post-merge protected-main discoverability and operational acceptance are required before Proposed decisions become Accepted/Implemented.
No version bump, release, deployment, legal-clearance or acquisition-ready claim is made.