Skip to content

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

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#103
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 711-line shell scanner and 452-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. 2086 lines go, 141 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: 2086
  • Added: 141

pm item

Quality gates

  • npm run check — pass
  • npm run docstring — pass (7 files, 47 declarations)
  • npm run coverage — pass (thresholds 70/80/80 met)
  • npm run verify:release-publish-attestation — pass
  • npm test — pass (114 tests)
  • npm run changelog:full — pass
  • npm run changelog:check — pass

Summary by Sourcery

Consume the canonical pm-ops attestation gate so release publishing uses the centrally maintained security checks.

Bug Fixes:

  • Close fail-open attestation paths by replacing the stale local scanner and verifier with the maintained canonical gate.

Enhancements:

  • Reduce the release attestation verifier to a thin launcher and re-export of the canonical pm-ops/attestation implementation.
  • Replace shell-model unit tests with convergence tests that verify canonical package consumption, runtime behavior, entry-point handling, and shebang behavior.

Build:

  • Upgrade the pm-ops dependency to version 2026.9.7.

Tests:

  • Update attestation coverage to validate consumption of the canonical gate rather than duplicate its shell-analysis implementation.

Chores:

  • Remove the vendored shell scanner, its associated tests, and over 2,000 lines of duplicated implementation and test code.

Summary by cubic

Replaces the vendored 711-line shell scanner and 452-line verifier with the canonical attestation gate from pm-ops, closing fail-open constructions the frozen copy still admitted.

Refactors

  • Deletes the local scanner; the verifier is now a thin launcher over pm-ops/attestation.
  • Tests assert this repo still consumes the canonical gate instead of re-testing the shell model.
  • Bumps pm-ops to 2026.9.7; quoted-metachar-value and single-quoted-metachar-value remain open there and close in a later version bump.
  • Fixes the launcher docstring's shebang claim: only a shebang naming a shell interpreter makes a file shell input, not any shebang, and tests now reproduce all four states.

Written for commit 2ae8254. Summary will update on new commits.

Review in cubic

This repository carried its own 711-line shell scanner and 452-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. 2086 lines go, 141 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 1 hour and 10 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: 3bf3465f-a822-4f7d-8381-6bdfa734697e


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 frozen, duplicated attestation gate and makes the release verifier a thin launcher over the canonical pm-ops/attestation implementation, with tests focused on dependency lineage and end-to-end gate behavior rather than duplicating shell-model coverage.

File-Level Changes

Change Details Files
Replaced the repository-owned attestation scanner and verifier with a dependency-backed launcher.
  • Deleted the 711-line shell scanner and its standalone implementation logic.
  • Upgraded pm-ops to ^2026.9.7 and refreshed the lockfile.
  • Imported and re-exported the canonical audit, verification, and reporting functions.
  • Kept process-entry detection, repository-root resolution, output forwarding, and exit-code behavior in the launcher.
  • Preserved the no-shebang constraint to prevent the launcher from being treated as a shell source.
scripts/shell-command-scan.ts
scripts/verify-release-publish-attestation.ts
package.json
package-lock.json
Changed tests from validating a local shell model to validating canonical implementation consumption and integration behavior.
  • Removed the scanner unit-test file and the former verifier's extensive shell-fixture suite.
  • Asserted the local scanner is absent and the launcher imports directly from pm-ops/attestation.
  • Asserted launcher exports are identical by reference to the dependency exports.
  • Retained behavioral checks for attested and unattested workflow publishes, repository verification, entry-point execution, and failing exit codes.
test/shell-command-scan.test.ts
test/verify-release-publish-attestation.test.ts
Recorded the security migration in project metadata and release documentation.
  • Added an Unreleased security changelog entry.
  • Added the associated PM work item and history records.
CHANGELOG.md
.agents/pm/chores/pm-linear-onxx.toon
.agents/pm/history/pm-linear-onxx.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 stale, vendored publish-attestation scanner with the canonical pm-ops/attestation implementation.

  • Removes the local shell scanner and its duplicated behavioral tests.
  • Converts the release verifier into a thin launcher that re-exports the canonical auditor.
  • Upgrades and locks pm-ops to 2026.9.7.
  • Adds convergence, launcher, rejection-path, and shebang-classification coverage.
  • Records the security change in the changelog and associated project-management history.

Confidence Score: 5/5

The PR appears safe to merge; the follow-up changes correct and directly test the launcher’s shebang documentation without introducing a new functional issue.

No accepted new findings or outstanding blocking failures remain. The verifier delegates to the locked canonical implementation, preserves failure reporting and exit behavior through the launcher, and tests both canonical identity and rejection of unattested publishes.

Important Files Changed

Filename Overview
scripts/verify-release-publish-attestation.ts Replaces the local verifier implementation with a thin launcher and direct re-exports from pm-ops/attestation.
test/verify-release-publish-attestation.test.ts Replaces duplicated scanner-model tests with package-identity, behavior, launcher, repository-gate, and shebang-classification checks.
scripts/shell-command-scan.ts Deletes the obsolete vendored shell scanner now supplied by the canonical package.
package.json Advances the canonical pm-ops dependency to the release containing the consolidated attestation gate.
package-lock.json Locks pm-ops 2026.9.7 with its registry URL and integrity digest.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Release[Release verification command] --> Launcher[scripts/verify-release-publish-attestation.ts]
    Launcher --> Canonical[pm-ops/attestation]
    Canonical --> Scan[Inspect tracked publish sources]
    Scan --> Decision{Every publish attested?}
    Decision -->|Yes| Pass[Exit successfully]
    Decision -->|No| Fail[Report failures and exit 1]
Loading

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

…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 are worth naming, because both were cases where the 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 133492c into main Sep 7, 2026
10 checks passed
@unbraind
unbraind deleted the build/consume-canonical-attestation-gate branch September 7, 2026 10:44
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