From b64a884295b948e64807eccee3e2616fa54b63ed Mon Sep 17 00:00:00 2001 From: Everest Date: Sun, 2 Aug 2026 06:08:13 +0800 Subject: [PATCH 1/2] docs(security): freeze external audit handoff --- .github/workflows/security-rc.yml | 4 +- audits/g4/AUDIT_HANDOFF.md | 131 +++++++++++ audits/g4/EXTERNAL_REVIEW_REQUEST.md | 5 + audits/g4/README.md | 12 +- audits/g4/handoff-manifest.json | 136 +++++++++++ package.json | 7 +- scripts/security-audit-handoff.mjs | 295 ++++++++++++++++++++++++ scripts/security-audit-handoff.test.mjs | 91 ++++++++ 8 files changed, 676 insertions(+), 5 deletions(-) create mode 100644 audits/g4/AUDIT_HANDOFF.md create mode 100644 audits/g4/handoff-manifest.json create mode 100644 scripts/security-audit-handoff.mjs create mode 100644 scripts/security-audit-handoff.test.mjs diff --git a/.github/workflows/security-rc.yml b/.github/workflows/security-rc.yml index 0a2e9f1..6444ad9 100644 --- a/.github/workflows/security-rc.yml +++ b/.github/workflows/security-rc.yml @@ -9,6 +9,8 @@ on: - "contracts/test/**" - "contracts/script/Deploy.s.sol" - "contracts/foundry.toml" + - "scripts/security-audit-handoff.mjs" + - "scripts/security-audit-handoff.test.mjs" - "scripts/security-rc-gate.mjs" - "scripts/security-rc-gate.test.mjs" - "package.json" @@ -46,7 +48,7 @@ jobs: - name: Install forge-std if: hashFiles('contracts/lib/forge-std/src/Test.sol') == '' run: cd contracts && forge install foundry-rs/forge-std@v1.16.2 --no-git - - name: Validate frozen scope and residual risks + - name: Validate frozen scope and audit handoff run: pnpm security:scope - name: Test the security gate run: pnpm security:test diff --git a/audits/g4/AUDIT_HANDOFF.md b/audits/g4/AUDIT_HANDOFF.md new file mode 100644 index 0000000..352367d --- /dev/null +++ b/audits/g4/AUDIT_HANDOFF.md @@ -0,0 +1,131 @@ +# ERC-8350 G4 External Solidity Audit Handoff + +Status: **scope and evidence package frozen; qualifying external report pending** + +This page is the entry point for an independent review of the ERC-8350 G4 release +candidate. It separates three things that must not be conflated: + +1. the exact Solidity source under review; +2. project-produced context and reproducible evidence; and +3. the reviewer-owned report and attestation still required to complete G4. + +Project-produced material is preparation, not an external audit. The release gate +continues to fail until a qualifying reviewer publishes and attests a report. + +## Cryptographic anchors + +| Item | Frozen value | +|---|---| +| Repository | `https://github.com/AwareLiquid/ERC-8350` | +| Candidate | `G4-rc1` | +| Source tag | `audit/g4-rc1-source` | +| Source commit | `af5a75fb2db532fd5603554083d8895a825c2de2` | +| Source tree | `f8d66277123391ea54de3f53842256e17e2a497e` | +| Scope manifest | [`scope.json`](scope.json) | +| Scope SHA-256 | `c9db7d27957c86386d8842687a2d009c0ca03bc864307e88cfed0ee25f3ae9bf` | +| Residual-risk register | [`residual-risks.json`](residual-risks.json) | +| Risk SHA-256 | `307a52eb0bf512aea25565bf604d92fc6b1b2d64eb910595568a5f0c04aa6019` | +| Evidence revision | `31f4fbb3732652884dac8f66fcc7a3655113c969` | +| Evidence tree | `b9baf6f6b4337b72d79c5343f5d178a4dc80f589` | + +The evidence revision is a descendant of the source commit and does not change any +in-scope file. It adds review preparation, tests, documentation, and release gates. +[`handoff-manifest.json`](handoff-manifest.json) pins every evidence file byte-for-byte. + +## Review map + +| Area | Primary material | What to establish | +|---|---|---| +| Registry | `contracts/src/reference/AgentMemoryStateRegistry.sol`, `IAgentMemoryState.sol`, `ECDSA.sol`, `IERC1271.sol` | Space registration and rotation cannot be taken over; transitions bind the correct Space, sequence, roots, commitments, Registry, and chain; every authorization path has the intended replay boundary | +| Commitment boundary | `PrivateCommitment.sol`, [`docs/threat-model.md`](../../docs/threat-model.md) | Raw memory, locator, salt, and witness material are not required on-chain; salted commitments do not overclaim truth, availability, or deletion | +| Formal extensions | `AuditGrant.sol`, `DeletionAttestation.sol`, `SpaceDescriptor.sol` | An extension cannot act on or emit authoritative-looking evidence for the wrong Space, transition, controller, grant, or auditor | +| Tests | Registry, authorization, extension, Golden Vector, and invariant suites listed in the manifest | Positive and negative paths are reproducible; the fixed cross-Space grant issue remains fixed; EOA, ERC-1271, malformed return, revert, and EIP-7702 fallback paths match the documented policy | +| Deployment | `contracts/script/Deploy.s.sol`, `contracts/foundry.toml`, [`docs/deployment.md`](../../docs/deployment.md) | Compiler settings and constructor wiring match the scope; experimental `MemoryMarket` is not deployed; legacy Sepolia addresses are not mistaken for the corrected G4 candidate | +| Risks | [`residual-risks.json`](residual-risks.json), [`docs/security/residual-risks.md`](../../docs/security/residual-risks.md) | Every accepted non-Critical/High finding is explicit, owned, and has controls, deployment requirements, and a review trigger | + +`contracts/src/experimental/MemoryMarket.sol` is excluded. It transfers value, has +a separate trust model, and is not deployed by the scoped deployment script. + +## Security boundary + +The Registry proves that the configured authority approved an ordered commitment +transition. It does not prove that private memory is true, useful, retrievable, +conscious, legally owned, or deleted. Encryption profiles, witness distribution, +key custody, storage availability, and policy-specific proof systems remain outside +the G4 Solidity scope. + +Reviewers should pay particular attention to: + +- direct-call, ECDSA, ERC-1271, malformed-return, revert, and EIP-7702 fallback paths; +- EIP-712 domain, Registry, chain, Space, sequence, root, and config-nonce binding; +- calldata and event disclosure of commitments and metadata; +- the fixed `G4-INT-001` cross-Space `AuditGrant` issue; +- `G4-R-003`, which records the canonical EIP-7702-to-ECDSA fallback behavior; and +- deployment sentinels, casts, external calls, compiler settings, and extension + constructor wiring. + +## Reproduce the handoff + +```bash +git clone https://github.com/AwareLiquid/ERC-8350.git +cd ERC-8350 +git checkout security/g4-security-ready-rc +pnpm install --frozen-lockfile + +pnpm security:handoff +pnpm security:scope +pnpm security:test +pnpm check +pnpm conformance:clean +``` + +Review the frozen source independently from the evidence branch: + +```bash +git worktree add ../ERC-8350-g4-source audit/g4-rc1-source +cd ../ERC-8350-g4-source/contracts +forge test -vv +``` + +The internal-review record reports 48 Foundry tests. The evidence revision then adds +three authorization-boundary regressions, so the current packaged checkout reports +**51 passed, 0 failed**, including 256 invariant runs and 128,000 handler calls. +Reviewers must rerun the commands and publish their own methods and results; this +project-produced result is not a substitute for independent reproduction. + +## Deployment evidence + +There is **no deployment of the corrected G4 candidate**. The Sepolia deployment +documented in `docs/deployment.md` predates `G4-INT-001`. Its Registry can be used to +check stable typehashes, but its `AuditGrant` is deprecated and must not be treated as +evidence for the corrected extension. A replacement deployment remains blocked until +the external review is accepted. + +## Required reviewer output + +The reviewer must publish a report in a repository controlled by the reviewer or +review organization and provide: + +1. identity, relevant experience, method, tools, compensation, prior involvement, + and conflict disclosure; +2. the exact source commit, tree, scope digest, and risk-register digest above; +3. stable finding IDs, severities, dispositions, rationales, and fix verification; +4. an assessment of every accepted residual risk; +5. zero unresolved Critical or High findings; +6. the report bytes and SHA-256; and +7. a full-commit attestation using the statement required by + `external/external-review.template.json`. + +The final report bytes belong under `external/reports/`, accompanied by a completed +`external/external-review.json`. Only then may `pnpm security:rc` pass, subject to +human maintainer review of reviewer identity, independence, method, and report quality. + +## Current external-review status + +The two duplicate public comments currently on +[issue #11](https://github.com/AwareLiquid/ERC-8350/issues/11) do not satisfy the +gate. They omit reviewer identity and reviewer-controlled report evidence, describe +only part of the ten-file scope, and cite extension APIs that are not present in the +frozen source. The exact discrepancies and requested corrections are recorded in the +[`review intake`](external/REVIEW_INTAKE_2026-07-31.md) and +[`reviewer follow-up`](external/REVIEWER_FOLLOWUP.md). diff --git a/audits/g4/EXTERNAL_REVIEW_REQUEST.md b/audits/g4/EXTERNAL_REVIEW_REQUEST.md index 1bb7c2f..0059d2f 100644 --- a/audits/g4/EXTERNAL_REVIEW_REQUEST.md +++ b/audits/g4/EXTERNAL_REVIEW_REQUEST.md @@ -14,6 +14,10 @@ required reviewer-owned evidence package is published. Public coordination issue: [AwareLiquid/ERC-8350#11](https://github.com/AwareLiquid/ERC-8350/issues/11). +The consolidated reviewer entry point is +[`AUDIT_HANDOFF.md`](AUDIT_HANDOFF.md); its +[`handoff-manifest.json`](handoff-manifest.json) pins the Registry, threat-model, +test, and deployment evidence without changing the frozen source target. ## Frozen target @@ -53,6 +57,7 @@ From the branch containing this audit packet: ```bash pnpm install --frozen-lockfile +pnpm security:handoff pnpm security:scope pnpm check cd contracts diff --git a/audits/g4/README.md b/audits/g4/README.md index 776dc34..23efe96 100644 --- a/audits/g4/README.md +++ b/audits/g4/README.md @@ -10,6 +10,12 @@ and evidence package require correction before acceptance. See the is accepted, `pnpm security:rc` and `pnpm release:check` fail by design, and no artifact from this branch may be described as security-ready or production-ready. +Start an independent review with the consolidated +[`AUDIT_HANDOFF.md`](AUDIT_HANDOFF.md). Its machine-readable +[`handoff-manifest.json`](handoff-manifest.json) pins the Registry context, +threat model, tests, deployment evidence, and gate material to an evidence +revision that does not alter the frozen Solidity scope. + - Implementation and evidence PR: [AwareLiquid/ERC-8350#10](https://github.com/AwareLiquid/ERC-8350/pull/10) - Public independent-review request: @@ -20,9 +26,10 @@ artifact from this branch may be described as security-ready or production-ready | Requirement | Evidence | Status | |---|---|---:| | Frozen Solidity target | [`scope.json`](scope.json), commit `af5a75f` | Pass | +| Reproducible reviewer handoff | [`AUDIT_HANDOFF.md`](AUDIT_HANDOFF.md), `pnpm security:handoff` | Pass | | Internal review and regression fix | [`INTERNAL_REVIEW.md`](INTERNAL_REVIEW.md) | Pass | | Remaining risks explicitly registered | [`residual-risks.json`](residual-risks.json) | Pass | -| Scope/evidence gate has adversarial tests | `scripts/security-rc-gate*.mjs` | Pass | +| Scope/evidence gates have adversarial tests | `scripts/security-rc-gate*.mjs`, `scripts/security-audit-handoff*.mjs` | Pass | | Independent external Solidity report | Preliminary email received; [`intake`](external/REVIEW_INTAKE_2026-07-31.md) | **Pending corrections/evidence** | | No unresolved Critical/High external findings | Requires accepted external report | **Pending** | | G4 full gate | `pnpm security:rc` | **Blocked by unaccepted review** | @@ -46,6 +53,9 @@ requires a new candidate plus external review of the changed target. # Passes now: validates exact source, scope, and risk registration. pnpm security:scope +# Passes now: validates all packaged Registry, threat, test, and deployment evidence. +pnpm security:handoff + # Passes now: attacks the gate's trust assumptions and failure paths. pnpm security:test diff --git a/audits/g4/handoff-manifest.json b/audits/g4/handoff-manifest.json new file mode 100644 index 0000000..df952c3 --- /dev/null +++ b/audits/g4/handoff-manifest.json @@ -0,0 +1,136 @@ +{ + "schema": "erc-8350/external-audit-handoff/v1", + "candidate": "G4-rc1", + "repository": "https://github.com/AwareLiquid/ERC-8350", + "source": { + "commit": "af5a75fb2db532fd5603554083d8895a825c2de2", + "tree": "f8d66277123391ea54de3f53842256e17e2a497e", + "ref": "refs/tags/audit/g4-rc1-source", + "scopeManifest": { + "path": "audits/g4/scope.json", + "sha256": "c9db7d27957c86386d8842687a2d009c0ca03bc864307e88cfed0ee25f3ae9bf" + }, + "residualRiskRegister": { + "path": "audits/g4/residual-risks.json", + "sha256": "307a52eb0bf512aea25565bf604d92fc6b1b2d64eb910595568a5f0c04aa6019" + } + }, + "evidenceRevision": { + "commit": "31f4fbb3732652884dac8f66fcc7a3655113c969", + "tree": "b9baf6f6b4337b72d79c5343f5d178a4dc80f589", + "relationship": "descendant-with-no-in-scope-source-changes" + }, + "evidence": [ + { + "category": "security-policy", + "path": "SECURITY.md", + "sha256": "a8b3a030d3fe10b9e6753cedb921ab0f8af7b32c23068ccbaab8cbce162f66bc" + }, + { + "category": "registry-architecture", + "path": "docs/architecture.md", + "sha256": "cfd59b8afeed99b1207c8261c6f117974e783db615c307145fb7d61f59801b72" + }, + { + "category": "threat-model", + "path": "docs/threat-model.md", + "sha256": "c518f9c048557d33c7fc3e148e28e1441df9120def6a66daff9cfbb7c183d34f" + }, + { + "category": "deployment-evidence", + "path": "docs/deployment.md", + "sha256": "19f63de3a3ae6688dccd5654adddf21865855c763170243fc46b2544c48ce1c9" + }, + { + "category": "residual-risk-narrative", + "path": "docs/security/residual-risks.md", + "sha256": "c68527e32165b33decc066e14912f8603762ec012e6dc8beda6f62c3fb2a8290" + }, + { + "category": "internal-review", + "path": "audits/g4/INTERNAL_REVIEW.md", + "sha256": "0d619cf7056b9cc6ac352695a6ae0aa8df56bdd37763745bede98bfd582f5c01" + }, + { + "category": "static-analysis", + "path": "audits/g4/internal/slither-summary.json", + "sha256": "f5fd5dad6acec9bb31b56b4ad2b283e9e46104dea5789557c02b3439adaf6bd4" + }, + { + "category": "golden-vector", + "path": "test-vectors/v1.json", + "sha256": "09b865b082b198606b0b005eec0df995d277f76f1e6b06550907472a9dcd2872" + }, + { + "category": "registry-tests", + "path": "contracts/test/unit/AgentMemoryStateRegistry.t.sol", + "sha256": "09a8d658f1e24b598235ae808b59d7ec3086e38a8d68c06f4878ad206ed8a474" + }, + { + "category": "audit-grant-tests", + "path": "contracts/test/unit/AuditGrant.t.sol", + "sha256": "c409d590f94df5bd426a4980c957fe6b3656ce0a77598fc0afe3be19e3e4904b" + }, + { + "category": "authorization-boundary-tests", + "path": "contracts/test/unit/AuthorizationBoundary.t.sol", + "sha256": "17c2f802bf11126cc73ea132e4dee85e7ab989ea50573dda54a1d5e1e5132c21" + }, + { + "category": "deletion-attestation-tests", + "path": "contracts/test/unit/DeletionAttestation.t.sol", + "sha256": "68c3db12eb291668014d5639c9fbf04fd921bfd3868799719c83f3aed63d4340" + }, + { + "category": "space-descriptor-tests", + "path": "contracts/test/unit/SpaceDescriptor.t.sol", + "sha256": "27feb82310afe76b84fc7a307717cfb4458ec0913c81114ed5a6c25a71e7b332" + }, + { + "category": "solidity-golden-vector-tests", + "path": "contracts/test/unit/GoldenVector.t.sol", + "sha256": "2b01e3b599f622f01df2580aee7cfeefa4ce6be68548626c6720edca0c47beb4" + }, + { + "category": "state-machine-invariants", + "path": "contracts/test/invariant/StateMachineInvariant.t.sol", + "sha256": "e20069adbc3ca4c5da375dffadb8e0e78d241497b661277a6afd109f642c447d" + }, + { + "category": "release-gate", + "path": "scripts/security-rc-gate.mjs", + "sha256": "61f432ca4e33daa545a1fc3f31e5cbebdb4bb9a94ff772e43facc217493ed3ed" + }, + { + "category": "release-gate-tests", + "path": "scripts/security-rc-gate.test.mjs", + "sha256": "a664b6be9200a523484cebe2b48b045f2110c4978cdc6a9b0e61cfae3966b2dd" + } + ], + "reproduction": { + "install": "pnpm install --frozen-lockfile", + "handoff": "pnpm security:handoff", + "scope": "pnpm security:scope", + "gateTests": "pnpm security:test", + "workspace": "pnpm check", + "cleanCheckout": "pnpm conformance:clean", + "solidity": "cd contracts && forge test -vv" + }, + "deployment": { + "correctedCandidateDeployed": false, + "legacySepolia": { + "chainId": 11155111, + "sourceCommit": "100d6ba", + "registry": "0xDdf21937ba80b5fF973610877A0955b320C91241", + "auditGrant": "0x20145Ab83958CFB321221e8a8C68181C818241B2", + "auditGrantDeprecated": true + } + }, + "externalReview": { + "coordinationIssue": "https://github.com/AwareLiquid/ERC-8350/issues/11", + "implementationPullRequest": "https://github.com/AwareLiquid/ERC-8350/pull/10", + "requiredEvidenceSchema": "erc-8350/external-solidity-review/v1", + "requiredEvidencePath": "audits/g4/external/external-review.json" + } +} + diff --git a/package.json b/package.json index eba552c..211f141 100644 --- a/package.json +++ b/package.json @@ -27,9 +27,10 @@ "evidence:validate": "node scripts/external-reproduction-gate.mjs --validate-only", "evidence:test": "node --test scripts/external-reproduction-gate.test.mjs", "evidence:external": "node scripts/external-reproduction-gate.mjs", - "security:scope": "node scripts/security-rc-gate.mjs --scope-only", - "security:test": "node --test scripts/security-rc-gate.test.mjs", - "security:rc": "node scripts/security-rc-gate.mjs", + "security:handoff": "node scripts/security-audit-handoff.mjs", + "security:scope": "node scripts/security-rc-gate.mjs --scope-only && pnpm security:handoff", + "security:test": "node --test scripts/security-rc-gate.test.mjs scripts/security-audit-handoff.test.mjs", + "security:rc": "pnpm security:handoff && node scripts/security-rc-gate.mjs", "test": "pnpm test:ts && pnpm check:vectors && pnpm test:contracts", "check": "pnpm clean && pnpm build && pnpm -r --if-present typecheck && pnpm -r --if-present test && pnpm check:erc-assets && pnpm check:vectors && pnpm check:private-witness && pnpm evidence:validate && pnpm evidence:test && pnpm security:scope && pnpm security:test && pnpm format:check && pnpm test:contracts", "conformance:gate": "node scripts/conformance-gate.mjs", diff --git a/scripts/security-audit-handoff.mjs b/scripts/security-audit-handoff.mjs new file mode 100644 index 0000000..c678edb --- /dev/null +++ b/scripts/security-audit-handoff.mjs @@ -0,0 +1,295 @@ +#!/usr/bin/env node + +import { createHash } from "node:crypto"; +import { lstat, readFile, realpath } from "node:fs/promises"; +import path from "node:path"; +import process from "node:process"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +const scriptDirectory = path.dirname(fileURLToPath(import.meta.url)); +const repositoryRoot = path.resolve(scriptDirectory, ".."); +const defaultManifestPath = path.join( + repositoryRoot, + "audits", + "g4", + "handoff-manifest.json", +); + +const expected = { + repository: "https://github.com/AwareLiquid/ERC-8350", + candidate: "G4-rc1", + sourceCommit: "af5a75fb2db532fd5603554083d8895a825c2de2", + sourceTree: "f8d66277123391ea54de3f53842256e17e2a497e", + sourceRef: "refs/tags/audit/g4-rc1-source", + evidenceCommit: "31f4fbb3732652884dac8f66fcc7a3655113c969", + evidenceTree: "b9baf6f6b4337b72d79c5343f5d178a4dc80f589", + scopePath: "audits/g4/scope.json", + scopeDigest: "c9db7d27957c86386d8842687a2d009c0ca03bc864307e88cfed0ee25f3ae9bf", + riskPath: "audits/g4/residual-risks.json", + riskDigest: "307a52eb0bf512aea25565bf604d92fc6b1b2d64eb910595568a5f0c04aa6019", +}; + +const requiredEvidencePaths = new Set([ + "SECURITY.md", + "docs/architecture.md", + "docs/threat-model.md", + "docs/deployment.md", + "docs/security/residual-risks.md", + "audits/g4/INTERNAL_REVIEW.md", + "audits/g4/internal/slither-summary.json", + "test-vectors/v1.json", + "contracts/test/unit/AgentMemoryStateRegistry.t.sol", + "contracts/test/unit/AuditGrant.t.sol", + "contracts/test/unit/AuthorizationBoundary.t.sol", + "contracts/test/unit/DeletionAttestation.t.sol", + "contracts/test/unit/SpaceDescriptor.t.sol", + "contracts/test/unit/GoldenVector.t.sol", + "contracts/test/invariant/StateMachineInvariant.t.sol", + "scripts/security-rc-gate.mjs", + "scripts/security-rc-gate.test.mjs", +]); + +const requiredCommands = { + install: "pnpm install --frozen-lockfile", + handoff: "pnpm security:handoff", + scope: "pnpm security:scope", + gateTests: "pnpm security:test", + workspace: "pnpm check", + cleanCheckout: "pnpm conformance:clean", + solidity: "cd contracts && forge test -vv", +}; + +function assert(condition, message) { + if (!condition) throw new Error(message); +} + +function sha256(bytes) { + return createHash("sha256").update(bytes).digest("hex"); +} + +function isDigest(value) { + return typeof value === "string" && /^[0-9a-f]{64}$/u.test(value); +} + +function isSafeRelativePath(value) { + return ( + typeof value === "string" && + value.length > 0 && + !path.isAbsolute(value) && + !value.split(/[\\/]/u).includes("..") && + !value.includes("\0") + ); +} + +function resolveInside(relativePath, label) { + assert(isSafeRelativePath(relativePath), `${label} is not a safe relative path`); + const resolved = path.resolve(repositoryRoot, relativePath); + const relative = path.relative(repositoryRoot, resolved); + assert( + relative.length > 0 && !relative.startsWith("..") && !path.isAbsolute(relative), + `${label} escapes the repository`, + ); + return resolved; +} + +function git(args, options = {}) { + const result = spawnSync("git", args, { + cwd: repositoryRoot, + encoding: options.binary ? undefined : "utf8", + maxBuffer: 32 * 1024 * 1024, + }); + if (result.status !== 0) { + const stderr = Buffer.isBuffer(result.stderr) + ? result.stderr.toString("utf8") + : result.stderr; + throw new Error(`git ${args.join(" ")} failed: ${stderr.trim()}`); + } + return options.binary ? result.stdout : result.stdout.trim(); +} + +async function readJson(filePath, label) { + let bytes; + try { + bytes = await readFile(filePath); + } catch (error) { + if (error.code === "ENOENT") throw new Error(`${label} is missing: ${filePath}`); + throw error; + } + + try { + return { value: JSON.parse(bytes.toString("utf8")), bytes }; + } catch (error) { + throw new Error(`${label} is not valid JSON: ${error.message}`); + } +} + +async function verifyRegularFile(filePath, label) { + const metadata = await lstat(filePath); + assert(metadata.isFile(), `${label} must be a regular file`); + const canonicalRoot = await realpath(repositoryRoot); + const canonicalFile = await realpath(filePath); + const relative = path.relative(canonicalRoot, canonicalFile); + assert( + relative.length > 0 && !relative.startsWith("..") && !path.isAbsolute(relative), + `${label} resolves outside the repository`, + ); +} + +async function verifyPinnedFile(entry, evidenceCommit) { + assert(typeof entry.category === "string" && entry.category.length > 0, "Evidence category is required"); + assert(isDigest(entry.sha256), `Evidence ${entry.path} must pin a lowercase SHA-256`); + const filePath = resolveInside(entry.path, `Evidence ${entry.path}`); + await verifyRegularFile(filePath, `Evidence ${entry.path}`); + const currentBytes = await readFile(filePath); + assert( + sha256(currentBytes) === entry.sha256, + `Evidence ${entry.path} does not match its pinned SHA-256`, + ); + const committedBytes = git(["show", `${evidenceCommit}:${entry.path}`], { + binary: true, + }); + assert( + sha256(committedBytes) === entry.sha256, + `Evidence ${entry.path} does not match evidence revision ${evidenceCommit}`, + ); +} + +async function verifyAnchor(anchor, expectedPath, expectedDigest, evidenceCommit, label) { + assert(anchor?.path === expectedPath, `${label} path changed`); + assert(anchor?.sha256 === expectedDigest, `${label} digest changed`); + const filePath = resolveInside(anchor.path, label); + await verifyRegularFile(filePath, label); + const currentBytes = await readFile(filePath); + assert(sha256(currentBytes) === anchor.sha256, `${label} current bytes changed`); + const committedBytes = git(["show", `${evidenceCommit}:${anchor.path}`], { + binary: true, + }); + assert(sha256(committedBytes) === anchor.sha256, `${label} evidence bytes changed`); + return JSON.parse(currentBytes.toString("utf8")); +} + +async function main() { + const arguments_ = process.argv.slice(2); + let manifestPath = defaultManifestPath; + if (arguments_.length > 0) { + assert( + arguments_.length === 2 && arguments_[0] === "--manifest", + "Usage: security-audit-handoff.mjs [--manifest ]", + ); + manifestPath = path.resolve(repositoryRoot, arguments_[1]); + } + + const record = await readJson(manifestPath, "G4 audit handoff manifest"); + const manifest = record.value; + assert( + manifest.schema === "erc-8350/external-audit-handoff/v1", + `Unsupported handoff schema: ${manifest.schema}`, + ); + assert(manifest.candidate === expected.candidate, "Handoff candidate changed"); + assert(manifest.repository === expected.repository, "Handoff repository changed"); + + assert(manifest.source?.commit === expected.sourceCommit, "Frozen source commit changed"); + assert(manifest.source?.tree === expected.sourceTree, "Frozen source tree changed"); + assert(manifest.source?.ref === expected.sourceRef, "Frozen source ref changed"); + assert( + git(["rev-parse", `${manifest.source.ref}^{commit}`]) === manifest.source.commit, + "Frozen source tag does not resolve to the frozen commit", + ); + assert( + git(["rev-parse", `${manifest.source.commit}^{tree}`]) === manifest.source.tree, + "Frozen source commit does not resolve to the frozen tree", + ); + + const evidenceRevision = manifest.evidenceRevision ?? {}; + assert(evidenceRevision.commit === expected.evidenceCommit, "Evidence revision changed"); + assert(evidenceRevision.tree === expected.evidenceTree, "Evidence tree changed"); + assert( + evidenceRevision.relationship === "descendant-with-no-in-scope-source-changes", + "Evidence/source relationship changed", + ); + assert( + git(["rev-parse", `${evidenceRevision.commit}^{tree}`]) === evidenceRevision.tree, + "Evidence revision does not resolve to the pinned tree", + ); + assert( + git(["merge-base", manifest.source.commit, evidenceRevision.commit]) === + manifest.source.commit, + "Evidence revision is not a descendant of the frozen source", + ); + + const scope = await verifyAnchor( + manifest.source.scopeManifest, + expected.scopePath, + expected.scopeDigest, + evidenceRevision.commit, + "Scope manifest", + ); + await verifyAnchor( + manifest.source.residualRiskRegister, + expected.riskPath, + expected.riskDigest, + evidenceRevision.commit, + "Residual-risk register", + ); + + assert(Array.isArray(scope.inScope), "Scope manifest inScope must be an array"); + const scopedPaths = scope.inScope.map((entry) => entry.path); + const changedScopedFiles = git([ + "diff", + "--name-only", + `${manifest.source.commit}..${evidenceRevision.commit}`, + "--", + ...scopedPaths, + ]); + assert( + changedScopedFiles.length === 0, + `Evidence revision changes frozen source files: ${changedScopedFiles}`, + ); + + assert(Array.isArray(manifest.evidence), "Handoff evidence must be an array"); + const observedPaths = new Set(); + for (const entry of manifest.evidence) { + assert(isSafeRelativePath(entry?.path), "Handoff evidence contains an unsafe path"); + assert(!observedPaths.has(entry.path), `Duplicate evidence path: ${entry.path}`); + observedPaths.add(entry.path); + await verifyPinnedFile(entry, evidenceRevision.commit); + } + const missingPaths = [...requiredEvidencePaths].filter((item) => !observedPaths.has(item)); + const extraPaths = [...observedPaths].filter((item) => !requiredEvidencePaths.has(item)); + assert( + missingPaths.length === 0 && extraPaths.length === 0, + `Handoff evidence changed; missing=[${missingPaths.join(", ")}] extra=[${extraPaths.join(", ")}]`, + ); + + for (const [name, command] of Object.entries(requiredCommands)) { + assert(manifest.reproduction?.[name] === command, `Reproduction command ${name} changed`); + } + assert( + manifest.deployment?.correctedCandidateDeployed === false, + "Handoff must not claim a corrected G4 deployment before external review", + ); + assert( + manifest.deployment?.legacySepolia?.auditGrantDeprecated === true, + "Legacy Sepolia AuditGrant must remain explicitly deprecated", + ); + assert( + manifest.externalReview?.requiredEvidencePath === + "audits/g4/external/external-review.json", + "External-review evidence path changed", + ); + + console.log(`G4 audit handoff: PASS (${manifest.evidence.length} pinned evidence files)`); + console.log(`Frozen source: ${manifest.source.commit}`); + console.log(`Scope SHA-256: ${manifest.source.scopeManifest.sha256}`); + console.log(`Risk-register SHA-256: ${manifest.source.residualRiskRegister.sha256}`); + console.log(`Evidence revision: ${evidenceRevision.commit}`); + console.log(`Handoff manifest SHA-256: ${sha256(record.bytes)}`); + console.log("Corrected G4 deployment: NONE (legacy AuditGrant remains deprecated)"); +} + +main().catch((error) => { + console.error(`G4 audit handoff: FAIL\n${error.message}`); + process.exitCode = 1; +}); + diff --git a/scripts/security-audit-handoff.test.mjs b/scripts/security-audit-handoff.test.mjs new file mode 100644 index 0000000..8ff57c4 --- /dev/null +++ b/scripts/security-audit-handoff.test.mjs @@ -0,0 +1,91 @@ +import assert from "node:assert/strict"; +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +const scriptDirectory = path.dirname(fileURLToPath(import.meta.url)); +const repositoryRoot = path.resolve(scriptDirectory, ".."); +const scriptPath = path.join(scriptDirectory, "security-audit-handoff.mjs"); +const manifestPath = path.join( + repositoryRoot, + "audits", + "g4", + "handoff-manifest.json", +); + +async function canonicalManifest() { + return JSON.parse(await readFile(manifestPath, "utf8")); +} + +function runGate(customManifestPath) { + const arguments_ = customManifestPath + ? [scriptPath, "--manifest", customManifestPath] + : [scriptPath]; + return spawnSync(process.execPath, arguments_, { + cwd: repositoryRoot, + encoding: "utf8", + }); +} + +async function withManifest(mutator, assertion) { + const temporaryDirectory = await mkdtemp( + path.join(os.tmpdir(), "erc-8350-audit-handoff-"), + ); + try { + const manifest = await canonicalManifest(); + mutator(manifest); + const temporaryManifest = path.join(temporaryDirectory, "handoff.json"); + await writeFile(temporaryManifest, `${JSON.stringify(manifest, null, 2)}\n`); + assertion(runGate(temporaryManifest)); + } finally { + await rm(temporaryDirectory, { recursive: true, force: true }); + } +} + +test("accepts the canonical external-audit handoff", () => { + const result = runGate(); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /G4 audit handoff: PASS/u); +}); + +test("rejects a changed frozen source commit", async () => { + await withManifest( + (manifest) => { + manifest.source.commit = "0".repeat(40); + }, + (result) => { + assert.equal(result.status, 1); + assert.match(result.stderr, /Frozen source commit changed/u); + }, + ); +}); + +test("rejects a missing evidence file", async () => { + await withManifest( + (manifest) => { + manifest.evidence = manifest.evidence.filter( + (entry) => entry.path !== "docs/threat-model.md", + ); + }, + (result) => { + assert.equal(result.status, 1); + assert.match(result.stderr, /missing=\[docs\/threat-model\.md\]/u); + }, + ); +}); + +test("rejects treating the legacy AuditGrant as active", async () => { + await withManifest( + (manifest) => { + manifest.deployment.legacySepolia.auditGrantDeprecated = false; + }, + (result) => { + assert.equal(result.status, 1); + assert.match(result.stderr, /must remain explicitly deprecated/u); + }, + ); +}); + From b49781a57fdafd3293b76e32061069dd81dd4af0 Mon Sep 17 00:00:00 2001 From: Everest An Date: Mon, 3 Aug 2026 12:10:14 +1000 Subject: [PATCH 2/2] fix(security): verify handoff evidence from the committed bytes, not the checkout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same correction already applied to the G4 scope gate. verifyPinnedFile and verifyAnchor each hashed a pinned file twice: once from the working tree and once from the evidence revision. Only the second is meaningful — that is the revision a reviewer fetches. Reading the working tree makes the result depend on the checkout's line-ending normalisation, so on any CRLF checkout every pinned text file "changed" and `pnpm security:handoff` failed with "Scope manifest current bytes changed" while git itself reported no difference at all. Since security:handoff is wired into security:scope, and that into `pnpm check`, the whole check was red for Windows contributors for a reason that had nothing to do with the handoff. verifyAnchor had a second, worse version of the same problem: it asserted the digest of the committed bytes and then returned `JSON.parse(currentBytes)` — the working-tree read. It verified one artifact and handed back another. If they ever diverged, every downstream assertion would be made against bytes whose digest was never checked. It now parses the bytes it verified. The path-existence check is kept: a manifest naming a deleted file is a defect, not drift. Tests: 15/15 pass. The three that were failing all traced to this one early exit — two of them were asserting later error messages they never reached. --- audits/g4/AUDIT_HANDOFF.md | 2 +- audits/g4/handoff-manifest.json | 1 + scripts/security-audit-handoff.mjs | 28 ++++++++++++++++++++-------- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/audits/g4/AUDIT_HANDOFF.md b/audits/g4/AUDIT_HANDOFF.md index 352367d..13887b4 100644 --- a/audits/g4/AUDIT_HANDOFF.md +++ b/audits/g4/AUDIT_HANDOFF.md @@ -25,7 +25,7 @@ continues to fail until a qualifying reviewer publishes and attests a report. | Scope SHA-256 | `c9db7d27957c86386d8842687a2d009c0ca03bc864307e88cfed0ee25f3ae9bf` | | Residual-risk register | [`residual-risks.json`](residual-risks.json) | | Risk SHA-256 | `307a52eb0bf512aea25565bf604d92fc6b1b2d64eb910595568a5f0c04aa6019` | -| Evidence revision | `31f4fbb3732652884dac8f66fcc7a3655113c969` | +| Evidence revision | `31f4fbb3732652884dac8f66fcc7a3655113c969` (`refs/tags/audit/g4-rc1-evidence`) | | Evidence tree | `b9baf6f6b4337b72d79c5343f5d178a4dc80f589` | The evidence revision is a descendant of the source commit and does not change any diff --git a/audits/g4/handoff-manifest.json b/audits/g4/handoff-manifest.json index df952c3..c206af5 100644 --- a/audits/g4/handoff-manifest.json +++ b/audits/g4/handoff-manifest.json @@ -17,6 +17,7 @@ }, "evidenceRevision": { "commit": "31f4fbb3732652884dac8f66fcc7a3655113c969", + "ref": "refs/tags/audit/g4-rc1-evidence", "tree": "b9baf6f6b4337b72d79c5343f5d178a4dc80f589", "relationship": "descendant-with-no-in-scope-source-changes" }, diff --git a/scripts/security-audit-handoff.mjs b/scripts/security-audit-handoff.mjs index c678edb..1e9759f 100644 --- a/scripts/security-audit-handoff.mjs +++ b/scripts/security-audit-handoff.mjs @@ -24,6 +24,7 @@ const expected = { sourceRef: "refs/tags/audit/g4-rc1-source", evidenceCommit: "31f4fbb3732652884dac8f66fcc7a3655113c969", evidenceTree: "b9baf6f6b4337b72d79c5343f5d178a4dc80f589", + evidenceRef: "refs/tags/audit/g4-rc1-evidence", scopePath: "audits/g4/scope.json", scopeDigest: "c9db7d27957c86386d8842687a2d009c0ca03bc864307e88cfed0ee25f3ae9bf", riskPath: "audits/g4/residual-risks.json", @@ -139,13 +140,12 @@ async function verifyRegularFile(filePath, label) { async function verifyPinnedFile(entry, evidenceCommit) { assert(typeof entry.category === "string" && entry.category.length > 0, "Evidence category is required"); assert(isDigest(entry.sha256), `Evidence ${entry.path} must pin a lowercase SHA-256`); + // The path must still exist here — a manifest naming a deleted file is a defect. const filePath = resolveInside(entry.path, `Evidence ${entry.path}`); await verifyRegularFile(filePath, `Evidence ${entry.path}`); - const currentBytes = await readFile(filePath); - assert( - sha256(currentBytes) === entry.sha256, - `Evidence ${entry.path} does not match its pinned SHA-256`, - ); + // Authoritative: the bytes committed at the evidence revision. That is what a + // reviewer fetches. Reading the working tree instead makes the answer depend on the + // checkout's line-ending normalisation, which has nothing to do with the handoff. const committedBytes = git(["show", `${evidenceCommit}:${entry.path}`], { binary: true, }); @@ -160,13 +160,14 @@ async function verifyAnchor(anchor, expectedPath, expectedDigest, evidenceCommit assert(anchor?.sha256 === expectedDigest, `${label} digest changed`); const filePath = resolveInside(anchor.path, label); await verifyRegularFile(filePath, label); - const currentBytes = await readFile(filePath); - assert(sha256(currentBytes) === anchor.sha256, `${label} current bytes changed`); const committedBytes = git(["show", `${evidenceCommit}:${anchor.path}`], { binary: true, }); assert(sha256(committedBytes) === anchor.sha256, `${label} evidence bytes changed`); - return JSON.parse(currentBytes.toString("utf8")); + // Parse the bytes that were just verified, not a second read of the working tree — + // otherwise the digest check and the value actually used could describe different + // files, and the check would be attesting to something it never inspected. + return JSON.parse(committedBytes.toString("utf8")); } async function main() { @@ -204,10 +205,21 @@ async function main() { const evidenceRevision = manifest.evidenceRevision ?? {}; assert(evidenceRevision.commit === expected.evidenceCommit, "Evidence revision changed"); assert(evidenceRevision.tree === expected.evidenceTree, "Evidence tree changed"); + assert(evidenceRevision.ref === expected.evidenceRef, "Evidence revision ref changed"); assert( evidenceRevision.relationship === "descendant-with-no-in-scope-source-changes", "Evidence/source relationship changed", ); + // The evidence revision must be anchored by a tag, for the same reason the frozen + // source is. A commit reachable only from a review branch stops being reachable the + // moment that branch is rebased or squash-merged — the object survives for a while, + // and a checkout that still has it locally reports PASS while a fresh clone cannot + // resolve it at all. Requiring the ref makes that failure impossible to reach by + // accident rather than merely unlikely. + assert( + git(["rev-parse", `${evidenceRevision.ref}^{commit}`]) === evidenceRevision.commit, + "Evidence revision tag does not resolve to the pinned evidence commit", + ); assert( git(["rev-parse", `${evidenceRevision.commit}^{tree}`]) === evidenceRevision.tree, "Evidence revision does not resolve to the pinned tree",