Skip to content

Label Audit Commands as Bash - #848

Merged
ptr727 merged 4 commits into
developfrom
fix/audit-bash-fences
Aug 19, 2026
Merged

Label Audit Commands as Bash#848
ptr727 merged 4 commits into
developfrom
fix/audit-bash-fences

Conversation

@ptr727

@ptr727 ptr727 commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • Label all Bash-dependent audit command blocks as bash.
  • Add regression coverage that rejects a POSIX sh label on these blocks.

Root Cause

The audit commands use process substitution, here-strings, and ANSI-C quoting, but their fence labels implied generic POSIX shell compatibility.

Validation

  • 729 Python tests
  • audit self-tests and spec validation
  • ruff, mypy, and coverage
  • actionlint, markdownlint, EditorConfig, CSpell, ShellCheck, and PSScriptAnalyzer

Follow-up to the suppressed Copilot finding on #846.

Copilot AI lite review requested due to automatic review settings August 19, 2026 17:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the audit documentation to correctly label Bash-specific command blocks as bash, and adds a regression test to prevent reintroducing POSIX sh code fences in AUDIT.md.

Changes:

  • Relabel selected AUDIT.md fenced command blocks from sh to bash.
  • Add a unit test that fails if AUDIT.md contains a POSIX sh fence label.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
AUDIT.md Relabel audit command fences to bash for Bash-only syntax.
scripts/tests/test_release_guards.py Add regression coverage preventing AUDIT.md from using ```sh fences.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/tests/test_release_guards.py Outdated
Copilot AI review requested due to automatic review settings August 19, 2026 17:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/tests/test_release_guards.py:66

  • The regression test will fail with an unhelpful message because it only reports the repeated string "sh" and not where it occurs in AUDIT.md. Recording line numbers (and catching "sh" with trailing info) makes the failure actionable.
    def test_audit_bash_blocks_are_not_labeled_as_posix_shell(self) -> None:
        audit_lines = (REPO / "AUDIT.md").read_text(encoding="utf-8").splitlines()
        mislabeled = [line for line in audit_lines if line.strip() == "```sh"]

        self.assertEqual([], mislabeled)

Copilot AI review requested due to automatic review settings August 19, 2026 17:28
@ptr727

ptr727 commented Aug 19, 2026

Copy link
Copy Markdown
Owner Author

Suppressed comments (1)

scripts/tests/test_release_guards.py:66

The regression test will fail with an unhelpful message because it only reports the repeated string "sh" and not where it occurs in AUDIT.md. Recording line numbers (and catching "sh" with trailing info) makes the failure actionable.

Fixed in d31bd90. The regression test now reports each matching line number and text, and catches both an exact sh fence label and one followed by additional info.

Review round: #848 (review)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/tests/test_release_guards.py:70

  • The new regression test is both over-broad and under-specific: it forbids any sh fence anywhere in AUDIT.md (which would block legitimate POSIX sh examples), and it still wouldn't catch a regression where a Bash-only block is mislabeled as shell. Consider asserting that any code fence containing Bash-only constructs (e.g., process substitution <(, here-strings <<<, ANSI-C quoting $'...') is labeled ```bash instead.
    def test_audit_bash_blocks_are_not_labeled_as_posix_shell(self) -> None:
        audit_lines = (REPO / "AUDIT.md").read_text(encoding="utf-8").splitlines()
        mislabeled = [
            (number, line)
            for number, line in enumerate(audit_lines, start=1)

Copilot AI review requested due to automatic review settings August 19, 2026 17:33
@ptr727

ptr727 commented Aug 19, 2026

Copy link
Copy Markdown
Owner Author

Suppressed comments (1)

scripts/tests/test_release_guards.py:70

The new regression test is both over-broad and under-specific: it forbids any sh fence anywhere in AUDIT.md (which would block legitimate POSIX sh examples), and it still would not catch a regression where a Bash-only block is mislabeled as shell. Consider asserting that any code fence containing Bash-only constructs is labeled bash instead.

Fixed in 6f72b23da7daa0534196d721dc064e23e975d5a8. The test now parses fenced blocks, permits ordinary POSIX examples, and reports sh or shell fence labels only when their body contains Bash-only syntax. Its diagnostic includes the opening fence line.

Review round: #848 (review)

@ptr727

ptr727 commented Aug 19, 2026

Copy link
Copy Markdown
Owner Author

Correction: the fixing commit in the preceding response is 6f72b23. The abbreviated 6f72b23 was correct; the manually expanded suffix was not.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@ptr727
ptr727 marked this pull request as ready for review August 19, 2026 18:20
@ptr727
ptr727 merged commit 1cd5b84 into develop Aug 19, 2026
8 checks passed
@ptr727 ptr727 mentioned this pull request Aug 19, 2026
ptr727 added a commit that referenced this pull request Aug 19, 2026
## Summary

- Promote fail-closed release validation and audit path discovery.
- Promote explicit Bash labels for executable audit commands.
- Promote the canonical PyPI artifact-name regression guard.
- Promote the executable Python entry point for the release-guard test.
- Promote the internally consistent missing-job audit fixture.

## Source

- #844 (`a08713a`)
- #847 (`af2c8ff`)
- #848 (`1cd5b84`)
- #849 (`87c3960`)
- #850 (`9b6c01f`)
- #851 (`bb4d603`)

## Validation

- 730 Python tests
- audit self-tests and spec validation
- ruff, mypy, and coverage
- actionlint, markdownlint, EditorConfig, CSpell, ShellCheck, and
PSScriptAnalyzer
- Copilot full-diff review on each feature pull request

Closes #842
Closes #843
Closes #845
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.

2 participants