feat(publish): select findings and verify publication payloads - #484
feat(publish): select findings and verify publication payloads#484mldangelo-oai wants to merge 9 commits into
Conversation
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review Please review the new exact head 0ae2930. The installed-package smoke caught an existing privacy contract: results must not echo the requested assignee identity. The follow-up removes that output while keeping the assignee bound into the digest. The rebuilt installed-package smoke now passes, including matching and mismatched digest checks; full and randomized suites are running. |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ae2930f16
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
zcrab-oai
left a comment
There was a problem hiding this comment.
Reviewed selected-only publication, finding and destination binding, keyed assignee commitments, and history validation.
|
@codex review Please review current head |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
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. |
|
@codex review Please review exact head |
|
@codex security review Please review exact head |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
kmbroai
left a comment
There was a problem hiding this comment.
Critical review
Reviewed head 4bfc090abf42e41f2d70f8eb246d98fd1370b6c1.
Recommendation: keep finding selection; the optional reviewed-payload check is also justified. No new blocking correctness defect found in this diff. Publishing a selected subset is a concrete workflow requirement. Binding a later publication to the exact preview is useful for automation, provided it remains distinct from authorization and remote verification.
Correctness
selectPublicationFindings and publicationPayloadDigest reject unknown IDs, deduplicate selection, and preserve the canonical publication order. The digest includes destination, scan identity, occurrence IDs, and the actual issue content. The expected-digest comparison runs before publication-store preparation or issue creation. Using the full scan when validating/persisting local history avoids mistaking a selected subset for the complete stored finding inventory.
Removing the wall-clock “Uploaded” field is necessary for stable previews; Linear already records creation time. The current assigned-preview HMAC also addresses the prior enumerable-assignee concern: a plain hash of a low-entropy email and otherwise disclosed fields would not hide the email. Credential rotation intentionally invalidates an assigned preview and is documented. Unassigned previews remain credential-independent.
Simplification and integration
Keep this a small deterministic comparison, not an approval-token service or persisted approval state machine. A direct digest equality check is sufficient here because the digest is caller-supplied approval data, not an authentication secret. The preview itself still contains sensitive finding content and must be kept private.
Coordinate composition with #486 before merging both. Selection, --skip-existing, and --expect-digest need an explicit ordering: the checked digest must describe the exact set intended for the subsequent mutation, while history validation must still receive the full scan. Add one combined-case regression when the branches meet rather than separate logic in each command. This is an integration requirement, not a reproduced defect in either isolated head.
Verification
Ran cli-publish, publication integration, preparation, and publisher suites: 117 passed, 0 failed, including changed payload/destination/assignee/credential rejection and selected publication behavior. Linux, Bun 1.3.14 / Node 22.13.1 with cached dependencies. I did not run the installed-package smoke or contact Linear; passing local digest checks does not establish remote permission or issue persistence.
Summary
Select findings for publication and require the same prepared payload that was
reviewed in a dry run. Both controls are opt-in; publishing still defaults to
all findings and new issues.
Changes
publish scan --finding FINDING_IDand SDKfindingIds.payloadDigest; accept optional--expect-digest DIGESTand SDK
expectedDigest. Omitting the expected digest skips enforcement.Assigned previews use HMAC-SHA-256 with the selected Linear API credential;
unassigned digests remain credential-independent.
selected findings. With
--skip-existing, the digest covers pending issues.inputs produce stable previews.
smoke-test variable extraction. Eight net lines removed during simplification.
preserving its protocol messages, timeout, exit-code and tool-name assertions.
Testing
On
ba069c3128cde53c2a560743891a135726c771bd:sandbox-blocked process-group check passed outside the sandbox.
selected/digest and direct CLI previews, credential locking, bundled plugin
files, bundled Codex and nested-worker execution without global Codex.
Final main refresh (
01bd062): 210 publication/CLI/tool-name tests passed;types/models, formatting, build, static package validation and full installed
smoke passed again, including selected/digest previews under the network guard.
Selection, digest and integration-test code are unchanged by the clean merge.
The bundle matches main's 0.1.60; a real cached-0.1.59 upgrade matched all 118
plugin files and preserved credentials.
The full suite and native Windows were not rerun. The earlier Windows tool-name
failure could not be reproduced locally. Its fixture now uses the asynchronous
process path and passes locally; native confirmation remains necessary.
New-head CI remains for the second pass.
fd98a90): package 0.1.21 includes the MCP launcher-permission fix; feature source and bundled payload are unchanged. Types/model generation, formatting, build, 238 focused tests, static artifact verification and full installed-package smoke passed, including MCP initialization. CI was not awaited.Risk and rollout
Selection and digest enforcement are opt-in, but publication results now require
payloadDigest, and issue descriptions omit Uploaded. Callers assigning an issuemust use the same assignee and API credential for preview and publication.
Changing either requires a new preview.
The digest confirms the local prepared request, not permissions or the final
remote representation. Keep saved previews private. No dependency, migration,
automatic retry, deduplication or package release is added.
Public disclosure review
New content uses generic descriptions and synthetic fixtures. Existing corporate
author metadata and restricted automated review links remain in public history,
so the second attestation is unchecked.