Skip to content

Keep reusable workflow operands verifiable by the canonical auditor - #147

Merged
unbraind merged 2 commits into
mainfrom
fix/reusable-shell-attestation-compatibility
Oct 18, 2026
Merged

unbraind merged 2 commits into
mainfrom
fix/reusable-shell-attestation-compatibility

Conversation

@unbraind

@unbraind unbraind commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Dependabot PR #135 fails this repository's own workflows pass the gate: the updated canonical pm-ops auditor reports 38 unresolved executable positions in the reusable workflows. The coverage-error lines elsewhere in that job are expected gate self-test output.

Express dynamic workflow data as operands to literal commands so its executable boundary is provable:

  • Validate whole remote tag names before binding associative-array elements through declare.
  • Validate numeric versions as NUL-delimited whole values through grep, rejecting multiline inputs while retaining numeric suffixes and annotated-tag behavior.
  • Populate literal Git pathspec data through printf/mapfile, and classify paths with conditional tests and explicit continuations.

The shared auditor remains the resolved package export, with every finding enforced. The publication scope and safety properties are exercised with real workflow shell, literal tracker names, runtime exclusions, malformed tags, multiline versions, and tag text that must remain data.

The overlay also exposed an untested artifact-time branch in its refreshed canonical prepare launcher. Added pack/publish omission-path tests that preserve Git config and attributes and pass against both canonical launcher generations. Focused launcher coverage is 100/100/100/100 with 10/10 tests.

Validation: Under the exact complete #135 overlay, npm run release:check and bun run release:check with isolated CI identity each pass 766/766 tests, zero failures/skips, with all 13 authored modules at exactly 100% statements, branches, functions and lines. Restored the original dependency pins and launcher, installed with npm ci, and passed 64/64 focused baseline compatibility tests. Linked pm commands pass 56/56; final strict pm health, changelog and privacy checks pass.

Applied the exact full #135 diff over this fix, including both dependency updates and the merge-driver launcher refresh, then installed it with npm ci. The applied diff is byte-identical to the PR diff (SHA-256 c72710900b72ae73365b959aaf184821d1d49417f64434509a0f120d31af3aea). Under that overlay, restoring only the original workflow shell reproduces all 38 findings and fails the repository regression; restoring the fix passes 1/1 with zero skips. Focused release-tag tests pass 42/42 and literal-scope tests pass 4/4. Dependency changes are temporary proof inputs and are excluded from this PR.

The design and verification notes describe the operand-boundary property. Import fidelity PR #142 remains separate.

PM: pm-github-7c95.

Summary by Sourcery

Harden reusable workflows so every dynamic value remains verifiable as literal operand data by the canonical auditor.

Bug Fixes:

  • Make reusable workflow release-tag and version handling reject unsafe or multiline values while preserving annotated-tag resolution and numeric-version validation.
  • Prevent dynamic tracker and path data from being interpreted as executable workflow syntax, while retaining publication allowlists and runtime exclusions.

Enhancements:

  • Add documentation describing the operand-boundary attestation property and its verification against the canonical auditor.

Documentation:

  • Document the reusable workflow operand-boundary guarantees, retained safety properties, and verification coverage.

Tests:

  • Add workflow operand tests covering literal tracker names, pathspec exclusions, rejected paths, malformed and multiline versions, malicious tag text, and annotated tags.
  • Add artifact-time pack and publish tests verifying omission notices and unchanged Git configuration and attributes.

Chores:

  • Record the associated PM issue and history metadata.

Summary by cubic

Reworks the reusable workflow shell so the canonical auditor can prove every dynamic value is operand text, clearing the 38 unresolved executable positions the updated pm-ops auditor reports under Dependabot PR #135. No auditor finding is pinned, filtered, or exempted.

  • Validates whole remote tag names before binding associative-array elements through declare, and rejects multiline versions as NUL-delimited whole grep values.
  • Builds pathspec arrays with printf/mapfile and classifies paths with conditional tests instead of dynamic case arms.
  • Adds literal-operand tests and pack/publish omission regressions that leave Git config and attributes unchanged; a new doc describes the operand-boundary property.

The dependency changes from the overlay are temporary proof inputs and are excluded from this PR.

Written for commit 6a96a3b. Summary will update on new commits.

View guided diff Turn on auto-fix

CI receipt at 25835e48e31246a3f4b1e48db7bf362e5d453a67: required test (22) and test (26) pass, including native Bun acceptance; CodeQL and Semgrep pass. Skipped/neutral reviewer checks are not treated as approval.

@sourcery-ai

sourcery-ai Bot commented Oct 17, 2026

Copy link
Copy Markdown

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 3 days by commenting @sourcery-ai review. Upgrade to get a review now.

@greptile-apps greptile-apps 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: Advanced
  • Run ID: 1704f79c-ffa6-4d90-90c1-59c913996642

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 83a2df67-5579-4b9f-a03a-c04cf6e47dd1
📥 Commits

Reviewing files that changed from the base of the PR and between 247b95e and 25835e4.

📒 Files selected for processing: 9
  • .agents/pm/history/pm-github-7c95.jsonl
  • .agents/pm/issues/pm-github-7c95.toon
  • .github/workflows/pm-github-closeout.yml
  • .github/workflows/pm-github-sync.yml
  • docs/attestation-workflow-operands.md
  • test/attestation-workflow-operands.test.ts
  • test/import-gate.test.ts
  • test/prepare-merge-driver.test.ts
  • test/self-versioned-workflows.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Summary by CodeRabbit

  • Bug Fixes

    • Improved release-version validation in reusable workflows, including rejection of malformed tags and multiline version values.
    • Path matching now handles tracker paths containing spaces and special characters literally while preserving existing inclusion and exclusion rules.
    • Packaging and publishing can complete when the optional pm-ops tool is unavailable, with a single omission notice and no local Git configuration changes.
  • Documentation

    • Added guidance on workflow attestation checks and verification scenarios.
  • Tests

    • Expanded coverage for workflow path matching, version resolution, and packaging and publishing behavior.

Walkthrough

The sync and closeout workflows update release-version validation and path-handling shell forms. New and updated tests cover workflow operands, path rules, version resolution, and merge-driver omission behavior. Documentation and tracker records describe the reported findings and verification.

Changes

Reusable workflow shell compatibility

Layer / File(s) Summary
Release validation and path handling
.github/workflows/pm-github-sync.yml, .github/workflows/pm-github-closeout.yml
Both workflows validate release refs before associative-array use and use NUL-delimited grep checks for versions. They populate pathspecs with mapfile and replace case matching with conditional checks. Existing path inclusions, exclusions, and generated links remain unchanged.
Behavior tests and verification records
test/attestation-workflow-operands.test.ts, test/self-versioned-workflows.test.ts, test/import-gate.test.ts, test/prepare-merge-driver.test.ts, docs/attestation-workflow-operands.md, .agents/pm/issues/pm-github-7c95.toon, .agents/pm/history/pm-github-7c95.jsonl
Tests cover literal pathspec operands, accepted and rejected paths, version-resolution cases, and merge-driver omission during pack and publish. Documentation and tracker records describe the findings and verification.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related PRs

  • unbraind/pm-github#57: Introduced the provenance attestation verifier and release and CI gates that scan shell command invocations.
Merge Risk: ⚪ Minimal · up to 25835

No actionable issue remains identified; the change is mergeable after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: keeping reusable workflow operands verifiable by the canonical auditor.
Description check Passed The description directly explains the workflow changes, verification goals, tests, compatibility results, and auditor findings addressed by the pull request.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (5 skipped: 5 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (5 skipped: 5 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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 Oct 17, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR rewrites dynamic shell handling in both reusable workflows into auditor-verifiable operand boundaries, preserving release resolution and publication safety while adding focused adversarial coverage, documenting exact-overlay verification, and testing artifact-time launcher omission behavior.

Sequence diagram for verifiable reusable workflow inputs

sequenceDiagram
    participant Workflow as ReusableWorkflow
    participant Shell as WorkflowShell
    participant Git as GitRemote
    participant Auditor as CanonicalAuditor

    Workflow->>Git: git ls-remote
    Git-->>Shell: remote_tags
    Shell->>Shell: printf
    Shell->>Shell: grep -zEq
    alt valid release tag
        Shell->>Shell: declare
        Shell->>Shell: grep -zEq
        Shell->>Workflow: PM_GITHUB_VERSION
    else malformed or multiline tag/version
        Shell-->>Workflow: reject input
    end
    Auditor->>Workflow: verify literal command operands
Loading

Flow diagram for literal publication scope enforcement

flowchart TD
    A[Build sync_scope with printf and mapfile] --> B[Read changed paths]
    B --> C{check_sync_path}
    C -->|runtime exclusion| D[Reject path]
    C -->|tracker path or CHANGELOG.md| E[Continue publication]
    C -->|other path| D
    E --> F{Changed tracker item file}
    F -->|toon path| G[Generate item links]
    F -->|other allowed path| H[No item link generated]
Loading

File-Level Changes

Change Details Files
Refactor reusable workflow shell so dynamic values remain explicit operands that the canonical auditor can verify without weakening any findings.
  • Validate complete release-tag references before associative-array binding via literal declare commands.
  • Use NUL-delimited grep validation for versions and preserve annotated-tag and numeric-suffix behavior while rejecting multiline values.
  • Construct Git pathspec arrays with printf/mapfile and replace dynamic case patterns with explicit conditional classifications.
  • Apply the same operand-boundary fixes consistently to sync and closeout workflows.
.github/workflows/pm-github-sync.yml
.github/workflows/pm-github-closeout.yml
Add focused regression coverage for operand safety, workflow publication scope, and malicious or malformed release inputs.
  • Verify literal tracker names, spaces, glob characters, exclusions, runtime-path rejection, and adjacent-prefix handling.
  • Cover multiline version rejection, annotated tag behavior, and tag text that must remain data.
  • Update workflow source assertions for the revised literal path pattern.
test/attestation-workflow-operands.test.ts
test/self-versioned-workflows.test.ts
test/import-gate.test.ts
Document the auditor compatibility design and verification strategy, including validation against the complete Dependabot overlay.
  • Explain why each shell construct establishes a provable executable boundary.
  • Record preservation of publication, privacy, lease, branch, and release-tag safety properties.
  • Document overlay verification and separation from dependency and import-fidelity changes.
docs/attestation-workflow-operands.md
Expand merge-driver launcher tests for artifact-time pack and publish omission paths.
  • Pass the lifecycle command into the launcher test harness.
  • Verify missing development tools produce one explicit notice and no Git config, attributes, or driver mutations.
  • Exercise both pack and publish paths.
test/prepare-merge-driver.test.ts
Add PM issue tracking artifacts for the workflow operand work.
  • Record the issue definition and implementation history in repository PM metadata.
.agents/pm/issues/pm-github-7c95.toon
.agents/pm/history/pm-github-7c95.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

@unbraind

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

Please review head 25835e4, including outside-diff findings.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 48 minutes.

@unbraind

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

Please review head 25835e4, including outside-diff findings.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@greptile-apps greptile-apps 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

@unbraind
unbraind merged commit ec21772 into main Oct 18, 2026
9 checks passed
@unbraind
unbraind deleted the fix/reusable-shell-attestation-compatibility branch October 18, 2026 06:28
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