feat: normalize trusted-tool evidence and complete review workflow (TAB-21–29) - #48
Conversation
Plane-Work-Item: TAB-21 Entire-Checkpoint: 95aae876ef91
|
Linked to Plane Work Item(s) References This comment was auto-generated by Plane |
Plane-Work-Item: TAB-21 Entire-Checkpoint: 95aae876ef91
#49) * feat: rebuild provenance from pinned source snapshots Plane-Work-Item: TAB-22 Entire-Checkpoint: 95aae876ef91 * feat: explicit provenance verdicts and safe review (TAB-23–29) (#50) * feat: preserve safe source failure reasons in review packets Plane-Work-Item: TAB-23 Entire-Checkpoint: 95aae876ef91 * feat: exact-candidate packets, security, and review (TAB-24–29) (#51) * fix: enforce complete review packet bounds and candidate acceptance Plane-Work-Item: TAB-25 Validates TAB-24 candidate movement and unrelated-history rejection. Entire-Checkpoint: 7098fc8f9b88 * test: validate review packets with the native schema checker Plane-Work-Item: TAB-25 Entire-Checkpoint: 7098fc8f9b88 * feat: exact security evidence, review, and recovery (TAB-26–29) (#52) * feat: bind separate security checks to immutable review candidates Plane-Work-Item: TAB-26 Entire-Checkpoint: 3a4c8131eec1 * ci: require pinned provenance security scanners Plane-Work-Item: TAB-26 Entire-Checkpoint: 3a4c8131eec1 * feat: exact review results, safe publication, and recovery (TAB-27–29) (#53) * feat: show actionable provenance review and GitHub statuses Plane-Work-Item: TAB-27 Entire-Checkpoint: 3a4c8131eec1 * test: demonstrate failure recovery and secured review (TAB-28/TAB-29) (#54) * test: demonstrate provenance failure and clean-store recovery (TAB-28) Entire-Checkpoint: 3a4c8131eec1 * fix: review the secured lineage in the release demo (TAB-29) (#55) * test: generate unsafe token fixture at runtime (TAB-29) Entire-Checkpoint: 3a4c8131eec1 * fix: demonstrate review of the secured provenance lineage (TAB-29) Entire-Checkpoint: 3a4c8131eec1 --------- Co-authored-by: hudsonaikins-crown <hudson.aikins@blanoire.co> --------- Co-authored-by: hudsonaikins-crown <hudson.aikins@blanoire.co> --------- Co-authored-by: hudsonaikins-crown <hudson.aikins@blanoire.co> --------- Co-authored-by: hudsonaikins-crown <hudson.aikins@blanoire.co> --------- Co-authored-by: hudsonaikins-crown <hudson.aikins@blanoire.co> --------- Co-authored-by: hudsonaikins-crown <hudson.aikins@blanoire.co> --------- Co-authored-by: hudsonaikins-crown <hudson.aikins@blanoire.co>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e94954429
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const id = candidate.repositoryId.replace(/^github\.com\//, ""); | ||
| return parseGitHubRepositoryRemote(`https://github.com/${id}`); |
There was a problem hiding this comment.
Reject local IDs before creating GitHub statuses
When a repository has no remote, repositoryIdentity produces local/<hash> (and nonportable remotes can produce remote/<hash>), but prepending https://github.com/ makes either identifier parse as a valid GitHub owner/repository. Consequently, review emits links and status payloads for a nonexistent GitHub repository, and review-intent succeeds only for publication to fail later during origin verification. Exclude these reserved local/remote identities before constructing the GitHub URL.
Useful? React with 👍 / 👎.
| requireFact(Buffer.byteLength(JSON.stringify(result)) <= 65536); | ||
| return { ...result, digest: hash(result) }; |
There was a problem hiding this comment.
Count the digest in the security receipt size limit
When findings make the undigested result close to 65,536 bytes, this check passes and the subsequently appended digest pushes the returned receipt beyond the intended limit (the digest field adds roughly 76 serialized bytes). Such an oversized receipt can still be returned and attached because attachment does not recheck its size; construct the complete envelope first and apply the byte limit to that value.
Useful? React with 👍 / 👎.
| return [observe(context, "buildkite", buildId(context), "validation", status, [link("github", pullRequestId(context))], snapshot.capturedAt, | ||
| { refs: [{ kind: "manifest_digest", value: validation.suite.manifestDigest }, { kind: "validation_digest", value: validation.integrity.digest }] })]; |
There was a problem hiding this comment.
Reject validation receipts completed after their snapshot
When the supplied validation receipt has completedAt later than the Buildkite snapshot's capturedAt, validateValidationResult still accepts it and this line records the validation as observed at the earlier snapshot time. A malformed or future-dated passing receipt can therefore avoid the future-evidence check and produce passed validation evidence before it supposedly completed. Require the receipt to have completed no later than the source capture before deriving this observation.
Useful? React with 👍 / 👎.
| "checks": { | ||
| "type": "array", | ||
| "minItems": 4, | ||
| "maxItems": 4, | ||
| "items": { | ||
| "$ref": "#/$defs/check" |
There was a problem hiding this comment.
Require one check for each security category
Although the runtime importer requires the four checks in SECURITY_CHECKS order, this schema only requires four arbitrary check objects, so a receipt containing four secrets checks and no authorization, trust, or dependency check validates successfully. Schema-only consumers can consequently accept structurally incomplete security evidence; constrain the positions or otherwise require exactly one occurrence of every category.
Useful? React with 👍 / 👎.
| const review = snapshot.reviews.find((item) => item.id === context.selection.reviewId); | ||
| requireFact(review?.commit === context.candidate.headCommit, "record_missing"); | ||
| // GitHub approval alone does not prove which base/merge-base was reviewed. | ||
| const marker = `Tabellio-Candidate: ${context.candidate.id}`; | ||
| requireFact(typeof review.body === "string" && review.body.split(/\r?\n/).includes(marker), "review_binding_missing"); | ||
| const status = review.state === "approved" ? "passed" : review.state === "changes_requested" ? "failed" : "blocked"; |
There was a problem hiding this comment.
Consider every current review before reporting approval
When the selected review is approved but another review in the same snapshot requests changes for the same candidate, this lookup ignores the latter and emits only a passed review observation. The resulting review context can therefore be published green while GitHub still has live requested changes; aggregate all applicable current reviews, or verify GitHub's current review decision, before assigning the passed status.
Useful? React with 👍 / 👎.
TAB-21 through TAB-29: source readers normalize Plane, Git, Entire, GitHub, and Buildkite evidence into bounded, source-attributed records. Each reader isolates authentication, permission, missing-record, outage, and malformed-input failures. GitHub review markers bind the full candidate; Buildkite evidence also binds the validation manifest. Integrated child PRs add replay, explicit verdicts, safe packets, separate security, guarded publication, and 11 failure/recovery demo cases. The original eager Git collection issue is fixed by the integrated replay work.
Candidate
5e9495442903b97c1b254387bfd72272c98f30ca: required local validation, hosted product validation, and hosted Quality passed. Author review found no remaining code findings; the Plane bot reference is informational. PRs #49–55 are merged into this branch.Stacked on #47. Source fixture normalization and the matching blocked CLI/GitHub demo satisfy the explicit acceptance; complete live-provider provenance is not claimed. Foundation Buildkite still blocks main integration. Release and deployment remain separate decisions.