Skip to content

Consume the canonical attestation gate instead of carrying a copy of it - #88

Merged
unbraind merged 2 commits into
mainfrom
build/consume-canonical-attestation-gate
Sep 7, 2026
Merged

Consume the canonical attestation gate instead of carrying a copy of it#88
unbraind merged 2 commits into
mainfrom
build/consume-canonical-attestation-gate

Conversation

@unbraind

@unbraind unbraind commented Sep 7, 2026

Copy link
Copy Markdown
Owner

This repository carried its own 679-line shell scanner and 424-line verifier, duplicated from the same origin as every other package in the fleet. That gate decides whether an artefact may reach the registry, so a false pass is the failure that matters. The canonical implementation has had fifteen fail-open constructions found and closed in it; this vendored copy was frozen and still admitted them.

The scanner is deleted, the verifier becomes a thin launcher over pm-ops/attestation, and the test suite asserts consumption rather than re-testing the shell model. 2028 lines go, 144 arrive.

Measurement (check.sh)

  • BEFORE: nonliteral-overwrite,nonliteral-overwrite-cmdsub,bare-brace-scope-escape,subshell-scope-escape,cmdsub-in-command-position,backtick-command-position,unset-after-bind,if-branch-bind,semicolon-same-line-bind,quoted-metachar-value,single-quoted-metachar-value
  • AFTER: nonliteral-overwrite,nonliteral-overwrite-cmdsub,quoted-metachar-value,single-quoted-metachar-value

quoted-metachar-value and single-quoted-metachar-value are open in the published pm-ops@2026.9.7, not a convergence issue. A fix for them exists locally in pm-ops but has not been published; a version bump will close them.

Line counts

  • Removed: 2028
  • Added: 144

pm item

Quality gates

  • npm run check — pass
  • npm run docstring — pass (7 files, 83 declarations)
  • npm run coverage — pass (thresholds 81/77/88 met)
  • npm run verify:release-publish-attestation — pass
  • npm test — pass (263 tests)
  • npm run changelog:full — pass
  • npm run changelog:check — pass

Summary by Sourcery

Use the canonical pm-ops attestation gate instead of maintaining a duplicated local scanner and verifier.

Bug Fixes:

  • Close stale local attestation-gate behavior by consuming the canonical implementation with the latest fail-open fixes.

Enhancements:

  • Replace the vendored shell scanner and verifier with a thin launcher over pm-ops/attestation.
  • Change tests to verify canonical-gate consumption, rejection of unattested publishes, and successful auditing of this repository's workflows.

Build:

  • Update the pm-ops dependency to ^2026.9.7.

Documentation:

  • Add an Unreleased security changelog entry documenting use of the canonical attestation gate.

Tests:

  • Remove duplicated shell-model tests and add convergence tests that ensure the launcher resolves and executes the package-provided gate.

Chores:

  • Record the change in the project maintenance metadata.

Summary by cubic

Replaces the repo's frozen publish-attestation gate (a 679-line shell scanner plus a 424-line verifier) with the canonical pm-ops implementation, closing fail-open publish paths the stale copy still admitted. The verifier becomes a thin launcher, and tests now assert consumption of the canonical gate instead of re-testing the shell model.

Refactors

  • Deletes scripts/shell-command-scan.ts and its test; the verifier shrinks from 407 to 38 lines as a thin launcher over pm-ops/attestation.
  • The rewritten suite asserts the gate resolves, refuses an unattested publish, and passes this repo's own workflows; it also reproduces the four shebang states that decide whether the auditor treats a file as shell input.
  • Adds an Unreleased security changelog entry and a pm-graph chore record; 2028 lines removed, 144 added.

Dependencies

  • Bumps pm-ops from 2026.8.28 to ^2026.9.7.
  • quoted-metachar-value and single-quoted-metachar-value still fail check.sh on the published pm-ops@2026.9.7; the fix exists locally and a future bump closes them.

Written for commit b93699d. Summary will update on new commits.

Review in cubic

This repository carried its own 679-line shell scanner and 424-line verifier,
duplicated from the same origin as every other package in the fleet. That gate
decides whether an artefact may reach the registry, so a false pass is the
failure that matters - and the canonical implementation has now had fifteen
fail-open constructions found and closed in it, three of them introduced by the
fix for an earlier one. A copy frozen at any point in that sequence still admits
every construction closed after it, and nothing here would have said so.

The scanner is deleted, the verifier becomes a thin launcher over the published
pm-ops/attestation export. 2028 lines go, 144 arrive.

The suite changes shape deliberately. It no longer re-tests the shell model:
that suite belongs with the implementation, where a fix reaches every consumer
at once. It asserts instead that this repository is still a CONSUMER - no local
scanner, the gate importing the package rather than resolving any part of its
shell model locally, and the resolved gate still refusing an unattested publish
and passing on this repository's own workflows. Identity rather than
similarity, because a copy that behaves the same today is exactly what stops
behaving the same the next time the canonical implementation is fixed.

Deliberately carries no shebang. The auditor treats any file whose first two
bytes are a shebang as executable shell, so adding one pulls this file into
its own scan - and its prose, which necessarily names the command it guards,
then reads as an unattested invocation. The vendored predecessor had no
shebang for the same reason.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @unbraind, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 50 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 63e8bcb4-9ca2-4516-8e17-ee32ab390558


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR removes the duplicated 1,000+-line attestation implementation and its model-level tests, upgrades to the canonical pm-ops/attestation gate, and leaves a small launcher plus convergence tests that ensure the repository continues consuming the package and correctly propagates audit results.

Sequence diagram for release publish attestation verification

sequenceDiagram
    participant Release as Release process
    participant Launcher as Attestation launcher
    participant Gate as pm-ops/attestation
    participant Result as Process result

    Release->>Launcher: runIfMain()
    Launcher->>Gate: verify(root)
    Gate-->>Launcher: audit result
    Launcher->>Gate: report(result, stdout, exit)
    Gate-->>Result: success or exit code 1
    Result-->>Release: verification outcome
Loading

File-Level Changes

Change Details Files
Replace the vendored attestation scanner/verifier with a thin launcher that delegates all audit logic to the canonical pm-ops/attestation package.
  • Remove the local shell tokenizer, source discovery, publish detection, flag evaluation, reporting, and verifier implementation.
  • Upgrade pm-ops to the published 2026.9.7 range and re-export the canonical audit and verification functions.
  • Retain only repository-root selection, process-entry detection, report wiring, and exit-code handling in the launcher.
package.json
package-lock.json
scripts/shell-command-scan.ts
scripts/verify-release-publish-attestation.ts
Convert tests from validating a local shell model to validating canonical-package consumption and launcher behavior.
  • Delete the standalone shell scanner tests and the former extensive model-level verifier fixtures.
  • Assert the scanner copy is absent, the launcher imports the canonical module, and its re-exports preserve function identity.
  • Exercise canonical behavior for attested and unattested workflow publishes, repository verification, entry-point detection, and failing exit-code propagation.
test/shell-command-scan.test.ts
test/verify-release-publish-attestation.test.ts
Record the security change and associated project-management metadata.
  • Add an unreleased security changelog entry describing canonical gate consumption.
  • Add the PM chore and history records for the migration.
CHANGELOG.md
.agents/pm/chores/pm-graph-qg2n.toon
.agents/pm/history/pm-graph-qg2n.jsonl

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the repository’s vendored publish-attestation scanner with the canonical pm-ops/attestation implementation.

  • Deletes the duplicated shell scanner and its model-specific tests.
  • Converts the local verifier into a thin launcher and re-export layer.
  • Adds convergence tests for package identity, rejection of unattested publishing, current-workflow auditing, launcher execution, and shell-specific shebang handling.
  • Updates pm-ops and records the security change in project metadata and the changelog.

Confidence Score: 5/5

The PR appears safe to merge; the only previous finding was fully addressed and manually resolved, and no new actionable failure remains.

The corrected documentation now distinguishes shell from non-shell shebangs, and the focused regression test reproduces both accepted and rejected cases through the canonical verifier. The launcher imports and re-exports the package implementation directly, rejects an unattested fixture, and successfully audits the repository’s workflows.

Important Files Changed

Filename Overview
scripts/verify-release-publish-attestation.ts Replaces the vendored verifier with a thin launcher over the canonical attestation package and corrects the documented shebang behavior.
test/verify-release-publish-attestation.test.ts Replaces implementation-model tests with convergence, launcher, workflow, and reproduced shebang-behavior checks.
package.json Updates pm-ops to the release providing the canonical attestation export.
package-lock.json Locks the canonical gate dependency to pm-ops 2026.9.7 with registry integrity metadata.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Release verification script] --> B[pm-ops/attestation]
    B --> C[Scan tracked publish paths]
    C --> D{Every publish attested?}
    D -->|Yes| E[Report success]
    D -->|No| F[Report failures and set exit code 1]
Loading

Reviews (2): Last reviewed commit: "Correct a wrong claim about which sheban..." | Re-trigger Greptile

Comment thread scripts/verify-release-publish-attestation.ts Outdated
…nd reproduce it

The launcher docstring said the auditor treats any file whose first two bytes
are a shebang as executable shell. Reproduced against the real auditor, that is
false: only a shebang naming a shell interpreter makes the body shell input, so
#!/usr/bin/env node leaves this file unscanned while #!/bin/bash does not.

The same sentence is in all seven repositories carrying this launcher - the
error travelled with the copied text. The suite now reproduces all four states
rather than asserting any of them. Found by Greptile's unreproduced-claim rule.
@unbraind

unbraind commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Merging. Measured result for this repository, with the harness at scripts/attest-corpus/check.sh in the companion:

The vendored scanner is deleted and the gate is now the canonical pm-ops/attestation export. The corpus verdict drops to the same seven every converged repository reports — and that identity is the point, not the number. Those seven are open in the canonical implementation itself, not here, and unbraind/pm-ops#100 (merged) closes all seven, so one pm-ops release moves every consumer at once. Before convergence each repository carried its own set of nine to fifteen, no two alike, which no release could reach.

Two review findings changed the shape of this PR, and both were cases where a guard did not do what it claimed:

  • The consumer test did not test consumption. It asserted the package's exports are functions while its own comment claimed reference identity. A launcher that imported the package and re-exported a local wrapper would have passed — the exact re-fork the test exists to catch. Now asserted by reference, and proven non-vacuous: substituting a pass-through wrapper makes it fail.
  • The launcher docstring stated a wrong fact. It said any shebang makes a file shell input. Reproduced against the real auditor, only a shebang naming a shell interpreter does; #!/usr/bin/env node does not. The same wrong sentence was in all seven repositories carrying this launcher — propagated by copying the reference file, so the error travelled with the text. Corrected everywhere and now reproduced in the suite rather than asserted.

Thank you to the reviewers on this wave; both of the above came from review, not from the change's author.

@unbraind
unbraind merged commit cbe2f02 into main Sep 7, 2026
12 checks passed
@unbraind
unbraind deleted the build/consume-canonical-attestation-gate branch September 7, 2026 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant