fix(acquisition): authenticate data-room manifest evidence - #69
fix(acquisition): authenticate data-room manifest evidence#69seonghobae wants to merge 163 commits into
Conversation
📝 WalkthroughWalkthrough데이터룸 manifest, retained evidence, exact Git checkout 및 외부 receipt의 무결성 검증을 추가했다. 안전한 출력 경계와 PR stale-head 검사를 적용했다. Integrity 검증을 audit·release 흐름에 연결하고, 전송 evidence의 licensing/IP 검증과 회귀 테스트를 추가했다. Changes취득 데이터룸 무결성
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant PRWorkflow
participant ExactCheckout
participant GitPreflight
participant IntegrityAudit
participant RetainedEvidence
PRWorkflow->>ExactCheckout: SOURCE_SHA checkout
ExactCheckout->>GitPreflight: HEAD 및 tracked bytes 검증
GitPreflight->>IntegrityAudit: 인증된 checkout 전달
IntegrityAudit->>RetainedEvidence: receipt, artifact bytes 및 digest 검증
IntegrityAudit-->>PRWorkflow: integrity 결과와 gate 상태 반환
PRWorkflow->>GitPreflight: 검증 후 HEAD 재확인
PRWorkflow-->>PRWorkflow: stale PR head이면 실패
🚥 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 |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review of e76d51ada7d7560dde87ca7ecb5cd42acc149178: one current trust-boundary finding remains actionable. This is not an approval.
The new manifest and audit entrypoints bind the report to git rev-parse HEAD, but they execute and hash the current working tree without proving tracked source/evidence bytes still equal that commit. A dirty checkout can therefore keep source.commitSha=<HEAD> while changing a tracked catalog file, tracked evidence document, scripts/acquisition-data-room-integrity-audit.mjs, or scripts/lib/acquisition-data-room-integrity.mjs; the resulting manifest/audit can still describe itself as exact-commit-bound even though the bytes being evaluated are not from that commit. The static import also loads the verifier before any checkout-integrity refusal could run.
Please add RED regressions that keep HEAD fixed while mutating tracked evidence/control files and prove both acquisition:manifest and acquisition:integrity fail closed. Then make the entrypoints authenticate the tracked checkout before evidence use (configuration-isolated Git, bounded output, no network), and load the verifier only after that preflight or otherwise bind verifier/catalog bytes to the exact Git object. Keep intentionally untracked retained acquisition artifacts usable; the requirement is that tracked source/control/evidence cannot drift while still being labeled as the exact audited commit. Document the trust root and preserve the existing offline final-audit boundary.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
test/acquisition-transfer-rights.test.ts (1)
158-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win감사 출력 파일을 읽기 전에 존재 여부를 확인하세요.
감사가 출력을 쓰기 전에 종료하면
readFileSync가ENOENT로 실패합니다. 그러면 실제 원인인result.stderr가 테스트 보고서에 나타나지 않습니다. 진단이 어려워집니다.♻️ 제안 리팩터
const auditPath = join(outputDir, "acquisition-audit.json"); + if (!existsSync(auditPath)) { + throw new Error(`audit output missing: ${result.stderr || result.stdout}`); + } - const audit = JSON.parse(readFileSync(join(outputDir, "acquisition-audit.json"), "utf8")); + const audit = JSON.parse(readFileSync(auditPath, "utf8"));🤖 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/acquisition-transfer-rights.test.ts` around lines 158 - 162, Before reading acquisition-audit.json in the audit helper, check whether the file exists; if it is absent, return the result with transferCheck unset so result.stderr remains available for diagnostics. Preserve the existing JSON parsing and transfer evidence lookup when the file is present.test/acquisition-readiness-audit.test.ts (1)
10-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win감사 실행 헬퍼가 상속된
NOEMA_환경 변수를 제거하지 않습니다. 두 헬퍼 모두process.env를 통째로 전달하고 일부NOEMA_경로 변수만 덮어씁니다. 호스트나 CI가 나머지 변수를 설정하면 감사가 임시 fixture 루트 밖의 파일을 읽습니다. 테스트 결과가 환경에 따라 달라집니다.
test/acquisition-readiness-audit.test.ts#L10-L19:runAudit에서NOEMA_접두사 키를 제거한 기본 환경을 만든 뒤 인자로 받은env를 병합하세요.test/acquisition-transfer-rights.test.ts#L151-L157:runReportOnlyAudit에서 같은 방식으로NOEMA_접두사 키를 제거하고 필요한 세 변수만 설정하세요.🤖 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/acquisition-readiness-audit.test.ts` around lines 10 - 19, Inherited NOEMA_ environment variables make audit tests depend on the host environment. In test/acquisition-readiness-audit.test.ts lines 10-19, update runAudit to build its base environment from process.env with all NOEMA_-prefixed keys removed, then merge the supplied env overrides. Apply the same cleanup in test/acquisition-transfer-rights.test.ts lines 151-157 within runReportOnlyAudit, retaining only the three required NOEMA_ variables there.
🤖 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 `@scripts/acquisition-readiness-audit.mjs`:
- Around line 229-241: Replace the repository_rights existsSync/readFileSync
digest path in the rights validation block with validateDigestBoundArtifact,
reusing readStableFile’s bounded, non-following, stable-read behavior and the
MAX_DATA_ROOM_EVIDENCE_BYTES limit. Preserve the allowedRightsPaths check and
existing failure messages or equivalent digest-mismatch reporting.
- Around line 150-184: Update validateDigestBoundArtifact and the read path used
by readStableFile to validate every parent component from the repository root
before opening the artifact: reject symlink or non-directory parents and reject
any resolved path outside the repository root. Preserve the existing
canonical-path and digest checks, ensuring paths such as
assets/link/retained-artifact.json cannot be read through a symlinked
intermediate directory.
In `@test/acquisition-readiness-audit.test.ts`:
- Around line 129-152: Update the passingLicensingIp fixture to create each
referenced release artifact using writeDigestArtifact or equivalent file writes,
and derive every release_rights sha256 from the actual contents instead of
hard-coded values. Keep the artifact paths and passing transfer assertions
unchanged while synchronizing sbom, dependency_license_inventory, notice, and
provenance with files under artifacts/release.
---
Nitpick comments:
In `@test/acquisition-readiness-audit.test.ts`:
- Around line 10-19: Inherited NOEMA_ environment variables make audit tests
depend on the host environment. In test/acquisition-readiness-audit.test.ts
lines 10-19, update runAudit to build its base environment from process.env with
all NOEMA_-prefixed keys removed, then merge the supplied env overrides. Apply
the same cleanup in test/acquisition-transfer-rights.test.ts lines 151-157
within runReportOnlyAudit, retaining only the three required NOEMA_ variables
there.
In `@test/acquisition-transfer-rights.test.ts`:
- Around line 158-162: Before reading acquisition-audit.json in the audit
helper, check whether the file exists; if it is absent, return the result with
transferCheck unset so result.stderr remains available for diagnostics. Preserve
the existing JSON parsing and transfer evidence lookup when the file is present.
🪄 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: 5784faf1-6779-4835-82fd-34dafe3ff238
📒 Files selected for processing (4)
docs/evidence-templates/transfer-evidence.example.jsonscripts/acquisition-readiness-audit.mjstest/acquisition-readiness-audit.test.tstest/acquisition-transfer-rights.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/acquisition-transfer-rights.test.ts`:
- Around line 257-282: Update the retained-evidence path validation used by the
transfer-rights audit to fail closed on symlinks: before validating
release_rights.sbom, reject any symlink in every parent path component; before
validating repository_rights.path, reject a symlink at the leaf path. Apply the
corresponding coverage in test/acquisition-transfer-rights.test.ts ranges
257-282 and 284-304, ensuring both paths cannot resolve to external bytes
through parent or leaf symlinks.
🪄 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: b686fd96-f559-458c-882b-e97c84ae87fb
📒 Files selected for processing (2)
test/acquisition-readiness-audit.test.tstest/acquisition-transfer-rights.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/acquisition-readiness-audit.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/acquisition-readiness-audit.mjs (1)
306-313: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
artifact_rights_metadata를 필수 바인딩 및 내용 검증에 포함하세요.Line 306-313은 네 개의 release artifact만 검증합니다.
release.artifact_rights_metadata의 존재, 안정적 읽기, SHA-256, JSON identity를 검증하지 않습니다.따라서 metadata가 없는 경우와 custom 또는 unlicensed 결정에 OCI license claim이 있는 경우에도
licensingIp.pass가 통과할 수 있습니다.artifact_rights_metadata를 digest-bound artifact로 검증하고,repository,tag,commit_sha를release_rights와 정확히 비교하세요. custom 또는 unlicensed 결정이면 OCI license annotation claim도 거부하세요. 이는test/acquisition-transfer-rights.test.ts의 Line 239-278 회귀 테스트가 요구하는 계약입니다.🤖 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 `@scripts/acquisition-readiness-audit.mjs` around lines 306 - 313, Extend the release artifact validation flow around validateDigestBoundArtifact to require and validate release.artifact_rights_metadata, including stable reading, SHA-256 verification, and JSON identity checks. Compare its repository, tag, and commit_sha exactly with release_rights, and reject OCI license annotation claims when the decision is custom or unlicensed, preserving the licensingIp.pass contract.
🤖 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.
Outside diff comments:
In `@scripts/acquisition-readiness-audit.mjs`:
- Around line 306-313: Extend the release artifact validation flow around
validateDigestBoundArtifact to require and validate
release.artifact_rights_metadata, including stable reading, SHA-256
verification, and JSON identity checks. Compare its repository, tag, and
commit_sha exactly with release_rights, and reject OCI license annotation claims
when the decision is custom or unlicensed, preserving the licensingIp.pass
contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dab22dad-3761-40c9-a77f-b328d774e02e
📒 Files selected for processing (3)
scripts/acquisition-readiness-audit.mjstest/acquisition-readiness-audit.test.tstest/acquisition-transfer-rights.test.ts
1 participant
Purpose
Hardens acquisition data-room and transfer-rights evidence so mutable, contradictory, parser-ambiguous or self-asserted metadata cannot create a false buyer PASS. It authenticates exact-release artifact rights metadata and rejects duplicate decoded JSON keys / malformed UTF-8 across acquisition evidence. It does not choose an outbound license or manufacture legal authority.
Fresh source identity
main.main:c85d710804139c0697d7ef8fa47d02b1389e6d84(fix(security): update transitive nanoid for CVE-2026-67213 #76 integrated;nanoid@3.3.17protected truth).fix/acquisition-manifest-integrity.0689fa6933dfc8db89abd053427a78a45455c86c.9adb784902cae19a150146a8fadd6a0491f67814.The old PR body treated #76 as unintegrated and #89 as the active package-manager dependency. Those facts are historical: #76 is protected truth and clean post-#76 PR #91 is now the current deterministic Node/npm/lockfile/install-script owner.
Acquisition integrity boundary
artifact_rights_metadata, and contributor/IP evidence remain separate authorities.Exact-release rights and parser ambiguity
The release-rights receipt is digest-bound to repository, full commit, immutable release tag and artifact identity. OCI license metadata is accepted only when it exactly matches an already authorized owner/legal SPDX expression; custom/unlicensed decisions cannot invent
LicenseRef-Proprietaryor another artifact-side legal posture.Authenticated JSON bytes are checked for duplicate decoded keys before parsing and decoded with fatal UTF-8. This applies to nested
artifact_rights_metadataand top-level acquisition JSON such as revenue, transfer, release-publication, saleable-readiness, data-room and package metadata. Last-key-wins ambiguity fails closed.Current package-manager dependency: #91
#69 overlaps the package-manager control plane in
.github/workflows/ci.yml,package.json,vitest.config.ts,test/workflow-readiness.test.ts, andCHANGELOG.md.#91 now owns exact Node
24.19.0/ npm11.17.0,strict-allow-scripts=true, reviewed lifecycle-script authority, schema-v2 live-base lockfile control and frozen install semantics on the clean post-#76 lineage. Replaying #69 without preserving #91 could improve acquisition integrity while regressing supply-chain authority.Safe order:
Do not copy #91 wholesale into this stale head, destructively rebase it, or transfer predecessor checks/reviews.
Historical exact-head evidence
For
0689fa6933dfc8db89abd053427a78a45455c86c, application CI31351055985reached 82 test files / 873 tests with configured statements/branches/functions/lines at 100%, then failed at the then-inherited nanoid audit; reviewer-ci31351056011and central Security Scan31351055984were terminal success. That audit failure is historical now that #76 is protected truth, but all of this evidence remains bound to the stale exact head and does not transfer to a future rebuilt successor.Protected integration / acquisition boundary
Issue #68 remains open because implementation evidence is not protected/release/acquisition acceptance. Before merge:
No version bump, release, deployment, audit waiver, protection bypass or acquisition-ready claim is made.
Related: #3, #5, #27, #29, #40, #68, #76, #77, #79, #91