Repository navigation
Add fail-closed app release attestation to continuous audit - #398
Conversation
--attestation <path> loads the user-written v1.5.0 attestation; any refusal yields appReleaseAttested=false with a named reason. Exit 0 iff appReleaseEligible. HQ stays all-false. Test-First: scripts/lib/release-attestation.test.mjs::attestation: missing file is refused with a named reason Test-First: scripts/lib/release-attestation.test.mjs::attestation: invalid JSON is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: non-object JSON is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: extra key is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: missing key is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: wrong release is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: rows must be exactly 1..26 Test-First: scripts/lib/release-attestation.test.mjs::attestation: bad decidedAt is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: empty decidedBy is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: continuousEnabled true is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: commit mismatch is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: dirty work tree is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: a non-repository is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: happy path with the attestation itself committed on top Test-First: scripts/lib/release-attestation.test.mjs::attestation: a later code commit invalidates it Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ab0bf7e04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let head; | ||
| try { | ||
| head = git(root, ['rev-parse', 'HEAD']).trim(); | ||
| if (git(root, ['status', '--porcelain']).trim() !== '') return no('work tree is not clean'); |
There was a problem hiding this comment.
The documented .pa/release_attestation_v1.5.0.json is covered by the repository's .pa/* ignore rule, so an untracked local file is invisible to git status --porcelain; when its commit equals HEAD, the later diff check is skipped and the mutable, never-committed file is accepted. This lets the release audit report eligibility without the durable user commit promised by the workflow, so verify that the path is tracked and that its blob matches HEAD (or explicitly unignore the canonical file).
AGENTS.md reference: AGENTS.md:L44-L46
Useful? React with 👍 / 👎.
| // exactly the commits that touch nothing but the attestation file. | ||
| let changed = null; | ||
| try { | ||
| changed = git(root, ['diff', '--name-only', `${data.commit}..${head}`]).split('\n').filter(Boolean); |
There was a problem hiding this comment.
git diff --name-only <attested>..<head> only compares the endpoint trees, so a code commit followed by a revert leaves only the attestation path in the net diff and is accepted. This contradicts the stated guarantee that every later code change invalidates the attestation and can bind release approval to a different commit history; require the attested commit to be an ancestor and inspect the paths touched by every intervening commit rather than only the final tree difference.
Useful? React with 👍 / 👎.
| const report = await buildReport(process.cwd(), options); | ||
| console.log(JSON.stringify(report, null, options.json ? 2 : 0)); | ||
| return report.continuousEligible && report.continuousReleaseEligible ? 0 : 1; | ||
| return report.appReleaseEligible ? 0 : 1; |
There was a problem hiding this comment.
With a valid app attestation and ready setup/runtime, this now returns 0 even though every continuous-mode evidence flag above remains false and both continuousEligible and continuousReleaseEligible are false. docs/development/CONTINUOUS.md:95-105 documents this command as the fail-closed continuous rollout preflight whose exit remains nonzero while either eligibility result is false, so any existing wrapper that checks only the process status can now treat a blocked continuous rollout as successful; expose app-release eligibility through a distinct explicit mode or command instead of changing the continuous gate's success condition.
AGENTS.md reference: AGENTS.md:L118-L123
Useful? React with 👍 / 👎.
| if (typeof data.decidedAt !== 'string' || !ISO_8601.test(data.decidedAt) || Number.isNaN(Date.parse(data.decidedAt))) { | ||
| return no('decidedAt must be an ISO 8601 timestamp'); |
There was a problem hiding this comment.
For syntactically shaped but impossible dates such as 2026-02-30T10:00:00Z, Date.parse normalizes the value to March 2 rather than returning NaN, so the supposedly strict attestation accepts and permanently records an invalid decision timestamp. Validate the parsed calendar components against the supplied components, or use an ISO parser that rejects out-of-range dates.
Useful? React with 👍 / 👎.
| const report = await buildReport(process.cwd(), options); | ||
| console.log(JSON.stringify(report, null, options.json ? 2 : 0)); | ||
| return report.continuousEligible && report.continuousReleaseEligible ? 0 : 1; | ||
| return report.appReleaseEligible ? 0 : 1; |
There was a problem hiding this comment.
This exit status is the only enforcement added, but neither the tag-producing scripts/release.cmd nor the tag-triggered .github/workflows/release.yml invokes this command or loads the attestation. A user can therefore run the existing release script and publish signed installers with the attestation missing or refused, despite the new documentation stating that v1.5.0 release approval exists only after this confirmation; make the tag/release path fail before publishing unless the attestation passes for the exact tagged commit.
AGENTS.md reference: AGENTS.md:L44-L46
Useful? React with 👍 / 👎.
|
Queued — the merge queue status continues in this comment ↓. |
Test-First: scripts/lib/release-attestation.test.mjs::attestation: an ignored untracked file is refused Test-First: scripts/lib/release-attestation.test.mjs::attestation: a non-ancestor commit is refused
Regression-For: af11952
Test-First: scripts/lib/release-attestation.test.mjs::attestation: hidden working-copy content cannot replace the committed attestation
Regression-For: 241835d
Cuarroc
added
the
review-ok
|
@Mergifyio queue |
Merge Queue Status
This pull request spent 13 minutes 58 seconds in the queue, including 13 minutes 42 seconds running CI. Required conditions to merge
|
Das Audit prüft die vom Nutzer selbst angelegte Freigabebestätigung für v1.5.0 strikt und verweigert die App-Freigabe bei jedem Fehler mit einem benannten Grund. Die Datei muss nun nachweislich als reguläre Datei committed sein, aus dem committed Git-Blob gelesen werden und auf einen Vorfahren von HEAD verweisen. Der hohe Reviewbefund zur zuvor möglichen unversionierten oder lokal ausgetauschten Bestätigung wurde mit Red-first-Regressionstests behoben.
Report
What changed
scripts/lib/release-attestation.mjs, a fail-closed loader for the user-owned v1.5.0 app-release attestation.--attestation <path>toscripts/continuous-audit.mjs; the command exits successfully exactly whenappReleaseEligibleis true.HEAD:<path>blob, compared it with the working copy, and required the attested commit to equal HEAD or be its ancestor with only the attestation changed afterward.docs/setup/release-attestation.md. Agents still never create the attestation.Evidence
6ab0bf7e04a6ea9c0edfb461a92174b9ff75e123; reviewed/fixed candidate:541b81109a9669704ba6e943ca7bc0b36b8fa9e5; merge base:5ef7e7dc021e77f1ded3f551e0a13086d3c3e808.node --test scripts/lib/release-attestation.test.mjs— 15 passed, 2 failed, exit 1 before the fix.node --test scripts/lib/release-attestation.test.mjs— 17 passed, 1 failed, exit 1 before the fix.node --test scripts/lib/continuous-readiness.test.mjs scripts/lib/release-attestation.test.mjs— 26/26 passed, exit 0.bash scripts/ci/red-first.sh— exit 0; all new exact test names and bothRegression-Forancestors verified.CARGO_TARGET_DIR=<warm-slot> CARGO_BUILD_JOBS=1 bash scripts/ci/gates.sh lane prepush— exit 0; frontend 485/485, HQ/Node 584/584, Rust nextest 1824/1824 with 17 skipped, clippy green.541b811— exit 0 with the same test totals; foreground push succeeded andgit ls-remotematched541b81109a9669704ba6e943ca7bc0b36b8fa9e5.Architecture drift
No drift found against
AGENTS.md,docs/decisions.md,docs/architecture-rules.md, or the existing script-layer Git helper pattern. The change introduces no frontend state, Tauri command/result shape, store/lock access, IPC name, or Rust-to-frontend event path.Review disposition
Tier A, two independent non-Claude reviewers. Round 1 candidate:
6ab0bf7e04a6ea9c0edfb461a92174b9ff75e123. Round 2 reviewed only the correction delta6ab0bf7..541b811; no code changed after that round.scripts/lib/release-attestation.mjs:20,38,42-54together with.gitignore:27, an ignored, untracked canonical file could pass when its JSON named HEAD;git statusstayed clean and the loader did not prove a committed blob. Countercheck reproduced this in an isolated repository, so the finding survived. Fixed red-first byaf11952,994b23c,241835d, and541b811: the path is explicitly trackable, must be a committed regular file, and its parsed working data must equal the committedHEAD:<path>blob. The siblingassume-unchangedreplacement route is covered by the exact testattestation: hidden working-copy content cannot replace the committed attestation.scripts/lib/release-attestation.mjs:42-54,git diff <commit>..HEADdid not establish ancestry and could accept an unrelated commit. Countercheck showed no outer gate supplied that proof. Fixed red-first byaf11952and994b23cwithgit merge-base --is-ancestorand the exact testattestation: a non-ancestor commit is refused.glm-5.2:cloud, reported modelglm-5.2, F1, medium — rejected. At originalscripts/continuous-audit.mjs:67(now line 74), exiting fromappReleaseEligibleinstead of the continuous-release conjunction is the explicit E20 package contract;continuous-readiness.mjsdeliberately separates app release from disabled Continuous release. Countercheck found no regression.scripts/continuous-audit.mjs:48-55, the reviewer proposed thatreadiness.attestations.appReleasemight be absent. Countercheck tracedevaluateContinuousReadiness, which synchronously and unconditionally creates that object, so the alleged dereference failure is not reachable.scripts/lib/release-attestation.mjs:12,38-50, suppressed Git stderr produces a stable named fail-closed reason. Countercheck found no safety or functional defect; this is diagnostic wording only.scripts/lib/release-attestation.mjs:42-54independently matched Codex C2 and was fixed byaf11952and994b23c.6ab0bf7..541b811— no remaining finding. The delta was checked for path confinement, regular committed-file provenance, blob binding, clean-tree behavior, ancestry, sibling bypasses, and architecture drift.scripts/lib/release-attestation.mjs:18-19, there is no dedicated outside-repository test. Countercheck found the guard straightforward and redundant with the committed-file gate; no bypass or bug was demonstrated, so red-first does not require a speculative test.scripts/lib/release-attestation.mjs:45-50, malformed committed JSON maps to the generic Git-state-unavailable reason. It remains fail-closed; countercheck found only imprecise diagnostics, not a safety or functional defect.scripts/lib/release-attestation.mjs:52-56, a malformed or unknown commit shares the ancestor-mismatch reason. It remains fail-closed; countercheck again found only diagnostic wording.No review finding remains open. No manual queue or merge action was taken.
NICHT ABGEDECKT
#[cfg(unix)]tests or the Linux clippy arm; the Linux half requires WSL2 on ext4.