[Security hardening] Add automated security audit workflow by PascalThuet · Pull Request #2442 · github/spec-kit

PascalThuet · 2026-05-02T06:39:31Z

Summary

Add a dedicated Security Audit workflow for dependency and static-analysis checks.
Audit committed hashed requirements on push, pull request, and manual dispatch so ordinary CI stays deterministic.
Verify committed audit requirements are refreshed when dependency inputs change, while keeping live dependency resolution for the scheduled audit across the supported Python and OS matrix.
Run Bandit for high-severity findings with a scoped baseline for the accepted shell-step B602 finding.
Pin all repository GitHub Actions to full commit SHAs.
Harden extension, preset, integration, and workflow downloads with bounded reads, strict HTTPS redirects, optional sha256 verification, bounded ZIP extraction, and source-path traversal guards.

Security context

This creates a repeatable CI baseline for dependency advisories and high-severity static-analysis issues while preserving a scheduled live-resolution signal for newly published dependency problems. It also closes similar supply-chain and archive-handling gaps found during follow-up review.

Closes #2438

Validation

git diff --check
uvx ruff check on all changed Python surfaces and tests
uv run python -m pytest tests/test_download_security.py tests/test_extensions.py tests/test_presets.py tests/integrations/test_integration_catalog.py tests/test_security_workflow.py tests/test_upgrade.py -q
uv run python -m pytest -q
uv pip compile pyproject.toml --extra test --universal --generate-hashes --quiet --no-header --output-file , compared with .github/security-audit-requirements.txt
uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r .github/security-audit-requirements.txt --progress-spinner off
uvx --from bandit==1.9.4 bandit -r src -lll --baseline .github/bandit-baseline.json

Copilot

Pull request overview

Adds a new GitHub Actions workflow to introduce automated security checks for the Python codebase, aiming to catch dependency advisories and high-severity static-analysis findings in CI alongside the existing test/lint workflows.

Changes:

Add .github/workflows/security.yml with a dedicated Security Audit workflow.
Run pip-audit on pushes to main, pull requests, a weekly cron, and manual dispatch.
Run Bandit against src/, temporarily skipping B602 pending the shell-step hardening tracked in #2440.

Show a summary per file

File	Description
`.github/workflows/security.yml`	New CI workflow that adds dependency-audit and static-analysis jobs for the Python project.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 1/1 changed files
Comments generated: 5

mnriem

Please address Copilot feedback. If not applicable, please explain why

PascalThuet · 2026-05-05T16:46:52Z

Addressed the Copilot feedback in the latest push:

Dependency auditing now exports the locked runtime + test extra dependency set with uv export --extra test --frozen --no-emit-project, then audits the generated requirements file.
pip-audit and Bandit are pinned via uvx --from (pip-audit==2.10.0, bandit==1.9.4).
Removed the global Bandit --skip B602; the existing shell execution call sites now use targeted # nosec B602 suppressions with local context.
Added CONTRIBUTING.md security-check commands so contributors can reproduce the workflow locally.

Local validation:

uv export --quiet --extra test --frozen --format requirements.txt --no-emit-project --output-file /tmp/spec-kit-audit-requirements.txt
uvx --from pip-audit==2.10.0 pip-audit -r /tmp/spec-kit-audit-requirements.txt --progress-spinner off
uvx --from bandit==1.9.4 bandit -r src -lll
git diff --check

Results: pip-audit reported no known vulnerabilities, Bandit reported no issues, and git diff --check passed.

PascalThuet · 2026-05-05T16:49:40Z

Added automated regression coverage for the security workflow in tests/test_security_workflow.py.

The new tests statically verify that:

dependency auditing exports the locked runtime + test extra requirements before running pip-audit;
pip-audit and Bandit remain pinned via uvx --from;
Bandit does not use a global --skip B602;
CONTRIBUTING.md documents the same security commands.

Validation after this commit:

uv run python -m pytest tests/test_security_workflow.py -q
uv export --quiet --extra test --frozen --format requirements.txt --no-emit-project --output-file /tmp/spec-kit-audit-requirements.txt
uvx --from pip-audit==2.10.0 pip-audit -r /tmp/spec-kit-audit-requirements.txt --progress-spinner off
uvx --from bandit==1.9.4 bandit -r src -lll
git diff --check

All passed locally.

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

src/specify_cli/init.py:414

This second # nosec B602 suppresses Bandit for the non-capturing branch as well, so neither path through run_command will ever raise B602 again. If the intent is only to defer the current finding until #2440, the skip needs to stay in the workflow/configuration layer rather than permanently muting this call site in source.

            # shell=True is only available to callers that opt in explicitly.
            subprocess.run(cmd, check=check_return, shell=shell)  # nosec B602

Files reviewed: 5/5 changed files
Comments generated: 5

mnriem · 2026-05-05T18:04:30Z

Please address Copilot feedback. If not applicable, please explain why. Note the shell step should indeed be ignored

mnriem

See comments above

PascalThuet · 2026-05-05T20:18:16Z

Addressed the follow-up review in e1e8051:

Removed --frozen from the dependency export because this repo intentionally does not commit uv.lock; the workflow and contributor docs now use the same clean-checkout-safe command.
Replaced source-level # nosec B602 suppressions with a CI-level Bandit baseline at .github/bandit-baseline.json.
Kept that baseline scoped to exactly one accepted finding: B602 in ShellStep, matching the maintainer note that the shell step should be ignored.
Hardened the generic run_command helper so shell=True is rejected instead of suppressed, which removes the similar non-shell-step B602 sink entirely.
Expanded tests/test_security_workflow.py to catch these regressions: no lockfile flags, pinned tools, no global --skip B602, one-entry Bandit baseline, no source # nosec B602, docs in sync, and run_command(shell=True) rejection.

Validation:

uv export --quiet --extra test --format requirements.txt --no-emit-project --output-file /tmp/spec-kit-audit-requirements.txt
uvx --from pip-audit==2.10.0 pip-audit -r /tmp/spec-kit-audit-requirements.txt --progress-spinner off
uvx --from bandit==1.9.4 bandit -r src -lll --baseline .github/bandit-baseline.json
uv run python -m pytest tests/test_security_workflow.py tests/test_workflows.py -q
uvx ruff check src/
git diff --check

I also verified the uv export command in a temporary clean directory without uv.lock; it succeeds before running the audit.

PascalThuet · 2026-05-05T20:27:09Z

One more small cleanup after re-review: pushed 6f1da27 to use uv pip compile pyproject.toml --extra test for the audit requirements instead of uv export.

Reason: it still audits the runtime + test extra dependency set, but it avoids both requiring and generating a project uv.lock, which makes the CI and CONTRIBUTING.md command cleaner for this repo.

Revalidated locally:

uv pip compile pyproject.toml --extra test --quiet --output-file /tmp/spec-kit-audit-requirements.txt
uvx --from pip-audit==2.10.0 pip-audit -r /tmp/spec-kit-audit-requirements.txt --progress-spinner off
uvx --from bandit==1.9.4 bandit -r src -lll --baseline .github/bandit-baseline.json
uv run python -m pytest tests/test_security_workflow.py -q
git diff --check

All passed.

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

tests/test_security_workflow.py:91

Bandit doesn't require the exact literal # nosec B602 to suppress this finding; plain # nosec, #nosec, and multi-ID forms also disable B602. This check leaves those suppression paths untested, so a future source-level bypass can slip in without failing CI.

        )

    def test_b602_is_not_suppressed_in_source(self):
        source_text = "\n".join(
            path.read_text(encoding="utf-8")
            for path in (REPO_ROOT / "src").rglob("*.py")
        )

Files reviewed: 5/5 changed files
Comments generated: 3

mnriem · 2026-05-05T20:34:15Z

Please address Copilot feedback

PascalThuet · 2026-05-05T20:45:51Z

Addressed the latest Copilot review in 129d19e:

Dependency audit now runs on the supported OS/Python matrix: ubuntu-latest and windows-latest across Python 3.11, 3.12, and 3.13.
The requirements output path in CI now uses ${{ runner.temp }}, and the contributor docs use a relative spec-kit-audit-requirements.txt path instead of /tmp, so the commands are Windows-compatible.
The security workflow regression test now verifies GitHub Actions are pinned to full 40-character commit SHAs.
The Bandit suppression guard now catches any # nosec form, not only the exact # nosec B602 spelling.
The dependency audit now compiles hashed requirements and runs pip-audit --disable-pip --require-hashes, avoiding a second pip resolution step.

Validation:

uv run python -m pytest tests/test_security_workflow.py -q
uv pip compile pyproject.toml --extra test --python-version 3.11 --generate-hashes --quiet --output-file /tmp/spec-kit-audit-py311.txt
uv pip compile pyproject.toml --extra test --python-version 3.12 --generate-hashes --quiet --output-file /tmp/spec-kit-audit-py312.txt
uv pip compile pyproject.toml --extra test --python-version 3.13 --generate-hashes --quiet --output-file /tmp/spec-kit-audit-py313.txt
uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r /tmp/spec-kit-audit-py311.txt --progress-spinner off
uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r /tmp/spec-kit-audit-py312.txt --progress-spinner off
uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r /tmp/spec-kit-audit-py313.txt --progress-spinner off
uvx --from bandit==1.9.4 bandit -r src -lll --baseline .github/bandit-baseline.json
git diff --check

All passed locally.

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 5/5 changed files
Comments generated: 5

mnriem

Please address Copilot feedback

PascalThuet · 2026-05-05T21:51:54Z

Addressed the latest Copilot review round in commit aa02062:

Push/PR/manual dependency audits now use committed hashed requirements at .github/security-audit-requirements.txt for deterministic CI.
Scheduled audits still perform live resolution across the supported Python/OS matrix.
CONTRIBUTING now runs pip-audit from the committed requirements file and documents how to refresh it without creating a root-level artifact.
The security workflow tests now assert triggers, deterministic audit behavior, action SHA pins, hashed committed requirements, and the absence of run_command(shell=...).
PR validation notes now match the Bandit baseline command used by CI.

Local validation passed:

git diff --check
uv run python -m pytest tests/test_security_workflow.py -q
uv run python -m pytest -q
uvx ruff check src tests/test_security_workflow.py
uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r .github/security-audit-requirements.txt --progress-spinner off
uvx --from bandit==1.9.4 bandit -r src -lll --baseline .github/bandit-baseline.json

The new GitHub workflow runs are currently waiting for fork PR approval (action_required).

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 6/6 changed files
Comments generated: 2

mnriem · 2026-05-05T22:03:01Z

Please address Copilot feedback

PascalThuet · 2026-05-05T22:06:27Z

Added a follow-up hardening commit: 1f85b92.

This addresses the similar risk surfaces we discussed after the Copilot review:

All repository workflows now pin third-party actions to full commit SHAs, guarded by tests/test_github_workflows.py.
Extension/preset downloads now use bounded reads and optional catalog sha256 verification.
Extension/preset ZIP installs now use a shared safe extractor with traversal, Windows-drive, symlink, file-count, per-file-size, and total-size checks.
Extension/preset/workflow catalog fetches now use strict HTTPS redirect handling for download/catalog paths.
Extension command source files and manifest-declared paths now reject traversal before registration.

Local validation passed:

git diff --check
uvx ruff check src tests/test_download_security.py tests/test_github_workflows.py tests/test_registrar_path_traversal.py tests/test_extensions.py tests/test_presets.py tests/test_security_workflow.py
uv run python -m pytest tests/test_download_security.py tests/test_github_workflows.py tests/test_registrar_path_traversal.py tests/test_extensions.py tests/test_presets.py tests/test_workflows.py -q
uv run python -m pytest tests/test_security_workflow.py -q
uv run python -m pytest -q (2781 passed, 34 skipped)
uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r .github/security-audit-requirements.txt --progress-spinner off
uvx --from bandit==1.9.4 bandit -r src -lll --baseline .github/bandit-baseline.json

The latest GitHub workflow runs are again waiting for fork PR approval (action_required).

Copilot

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 24/24 changed files
Comments generated: 5

mnriem · 2026-05-05T22:29:58Z

Please address Copilot feedback. Thank you once again for working through all these reviews!

PascalThuet · 2026-05-06T05:40:02Z

Addressed the latest Copilot review feedback in commit 4599155.

What changed:

Reject Windows drive-relative paths such as C:secret.md in preset/extension command paths and ZIP member names.
Use bounded reads for catalog JSON in extension, preset, and integration catalogs, plus remaining CLI download surfaces that still had raw response reads.
Add a committed requirements sync check for PR/push runs when dependency inputs change, using GitHub-provided diff SHAs with a conservative fallback.
Regenerate .github/security-audit-requirements.txt without the uv header so the workflow sync comparison is stable.
Add regression coverage for max ZIP entries, total uncompressed ZIP size, bounded catalog reads, stale audit requirements checks, and a source scan preventing new bare resp.read()/response.read() calls under src/specify_cli.

Validation run locally:

git diff --check
uvx ruff check on all changed Python surfaces and new tests
uv run python -m pytest tests/test_download_security.py tests/test_extensions.py tests/test_presets.py tests/integrations/test_integration_catalog.py tests/test_security_workflow.py tests/test_upgrade.py -q: 629 passed
uv run python -m pytest -q: 2797 passed, 34 skipped
uv pip compile ... --no-header compared cleanly against .github/security-audit-requirements.txt
pip-audit on the committed requirements: no known vulnerabilities found
Bandit with the committed baseline: no issues identified

I attempted to resolve the five Copilot threads directly, but the GitHub integration returned Resource not accessible by integration, so they will need to be marked resolved by someone with repository permissions.

Copilot

Copilot's findings

Files reviewed: 22/22 changed files
Comments generated: 0 new

mnriem

Please address workflow errors

Resolve conflicts in src/specify_cli/__init__.py and src/specify_cli/integrations/catalog.py: - __init__.py: drop local definitions of run_command, check_tool, is_git_repo, init_git_repo, handle_vscode_settings, merge_json_files, _locate_core_pack, _repo_root, _locate_bundled_extension, _locate_bundled_workflow, _locate_bundled_preset. These now live in the new _utils.py / _assets.py modules introduced on main and are re-imported at the top of __init__.py. Keep the security-hardening read_response_limited import. - _utils.py: apply the security hardening from this branch to the relocated run_command — drop the shell= parameter so subprocess is never invoked via /bin/sh. - integrations/catalog.py: keep both imports (read_response_limited from this branch + CatalogEntry/CatalogStackBase from main).

Six follow-on checks that lock in the hardening from this PR and add the surfaces it didn't cover: 1. ruff S602/S604/S605 in pyproject.toml — fail PRs that reintroduce subprocess shell=True. The intentional shell=True in the workflows shell step keeps its NOTE comment and gets an explicit `# noqa: S602` so the deviation is visible. 2. Bandit two-pass in security.yml — keep `-lll --baseline` blocking and add a non-blocking `-ll` informational pass so MEDIUM findings show in the job summary instead of accumulating silently. 3. Bandit baseline diff check — fail PRs that grow .github/bandit-baseline.json unless they carry the `security-baseline-change` label. New script in .github/scripts/check_bandit_baseline.py. 4. Secret scanning via detect-secrets — new `secret-scan` job in security.yml with a committed .secrets.baseline that whitelists the nine current findings (all SHA pins / docs examples / test fixtures; audited before commit). Drift fails the check. 5. shellcheck on scripts/bash/*.sh in lint.yml. Starts at --severity=error to catch real bugs; style (SC2155) can be tightened in a follow-up. 6. macos-latest added to the dependency-audit matrix in security.yml — aligns with test.yml's posture and catches platform-specific resolver surprises.

PascalThuet · 2026-05-14T06:32:56Z

Hi @mnriem — quick status update and a small ask.

Conflicts resolved (e869b19): rebased onto current main after the _utils.py / _assets.py / catalogs.py refactors. The hardening from this branch is preserved (notably run_command dropping shell= is now applied to the relocated function in _utils.py; read_response_limited kept).

Follow-on PR checks (b089f8b) — six guardrails so the hardening doesn't silently regress:

ruff S602/S604/S605 (subprocess shell=True) in pyproject.toml, with one explicit # noqa: S602 on the intentional shell step.
Bandit two-pass in security.yml: HIGH+baseline blocking, MEDIUM non-blocking informational.
Bandit baseline diff check — fails PRs that grow .github/bandit-baseline.json without a security-baseline-change label. New helper script.
detect-secrets secret-scan job + committed .secrets.baseline (9 audited false positives: SHA pins, docs examples, test fixtures).
shellcheck --severity=error on scripts/bash/*.sh in lint.yml.
macos-latest added to the dependency-audit matrix in security.yml.

Tests: 2968 pass locally on the merged tree.

Ask: the four workflows (CodeQL, Lint, Test & Lint Python, Security Audit) are sitting in action_required on the latest commit. Could you click Approve and run workflows when you have a moment? Happy to split (2) into a separate PR if you'd rather review the conflict resolution first.

Add automated security audit workflow

e4b38e7

PascalThuet marked this pull request as ready for review May 2, 2026 06:45

PascalThuet requested a review from mnriem as a code owner May 2, 2026 06:45

mnriem requested a review from Copilot May 4, 2026 12:39

Copilot started reviewing on behalf of mnriem May 4, 2026 12:40 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

Comment thread .github/workflows/security.yml Outdated

Comment thread .github/workflows/security.yml Outdated

Comment thread .github/workflows/security.yml Outdated

Comment thread .github/workflows/security.yml Outdated

Comment thread .github/workflows/security.yml Outdated

mnriem requested changes May 4, 2026

View reviewed changes

Address security audit review feedback

65b0dab

Add security workflow regression tests

397e0d7

PascalThuet requested a review from mnriem May 5, 2026 16:50

mnriem requested a review from Copilot May 5, 2026 17:32

Copilot started reviewing on behalf of mnriem May 5, 2026 17:33 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread .github/workflows/security.yml Outdated

Comment thread CONTRIBUTING.md Outdated

Comment thread tests/test_security_workflow.py Outdated

Comment thread src/specify_cli/workflows/steps/shell/__init__.py Outdated

Comment thread src/specify_cli/__init__.py Outdated

mnriem requested changes May 5, 2026

View reviewed changes

Address follow-up security workflow review

e1e8051

mnriem requested a review from Copilot May 5, 2026 20:22

Copilot started reviewing on behalf of mnriem May 5, 2026 20:22 View session

Use compile for security audit requirements

6f1da27

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread .github/workflows/security.yml Outdated

Comment thread CONTRIBUTING.md Outdated

Comment thread tests/test_security_workflow.py Outdated

Address latest security workflow review

129d19e

mnriem requested a review from Copilot May 5, 2026 20:47

Copilot started reviewing on behalf of mnriem May 5, 2026 20:48 View session

PascalThuet requested a review from mnriem May 5, 2026 20:50

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread .github/workflows/security.yml Outdated

Comment thread CONTRIBUTING.md Outdated

Comment thread tests/test_security_workflow.py

Comment thread src/specify_cli/__init__.py Outdated

Comment thread .github/workflows/security.yml Outdated

mnriem requested changes May 5, 2026

View reviewed changes

Address latest security audit review

aa02062

mnriem requested a review from Copilot May 5, 2026 21:56

Copilot started reviewing on behalf of mnriem May 5, 2026 21:56 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread .github/workflows/security.yml

Comment thread tests/test_security_workflow.py

Harden security-sensitive repository surfaces

1f85b92

mnriem requested a review from Copilot May 5, 2026 22:08

Copilot started reviewing on behalf of mnriem May 5, 2026 22:09 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread src/specify_cli/presets.py

Comment thread src/specify_cli/extensions.py

Comment thread src/specify_cli/presets.py

Comment thread .github/workflows/security.yml

Comment thread src/specify_cli/_download_security.py

Address remaining security review feedback

4599155

PascalThuet requested a review from mnriem May 6, 2026 10:04

mnriem requested a review from Copilot May 6, 2026 16:16

Copilot started reviewing on behalf of mnriem May 6, 2026 16:17 View session

Merge origin/main into security audit workflow

d5c5ce7

mnriem requested review from Copilot and removed request for Copilot May 11, 2026 13:05

Copilot started reviewing on behalf of mnriem May 11, 2026 13:06 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

mnriem requested changes May 11, 2026

View reviewed changes

PascalThuet requested a review from mnriem May 14, 2026 06:19

Conversation

PascalThuet commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Security context

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

PascalThuet commented May 5, 2026

Uh oh!

PascalThuet commented May 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem commented May 5, 2026

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

PascalThuet commented May 5, 2026

Uh oh!

PascalThuet commented May 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem commented May 5, 2026

Uh oh!

PascalThuet commented May 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

PascalThuet commented May 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

mnriem commented May 5, 2026

Uh oh!

PascalThuet commented May 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

PascalThuet commented May 2, 2026 •

edited

Loading