diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 272db6a6fe..5ecc25a398 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,4 +1,4 @@ -name: Lint +name: Lint & Security permissions: contents: read @@ -6,9 +6,14 @@ on: push: branches: ["main"] pull_request: + schedule: + - cron: "17 4 * * 1" + workflow_dispatch: jobs: - markdownlint: + checks: + name: Lint & dependency audit + if: ${{ github.event_name != 'schedule' }} runs-on: ubuntu-latest env: DOC_GLOBS: | @@ -27,11 +32,31 @@ jobs: workflows/*.md steps: - name: Checkout + id: checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - fetch-depth: 1 + fetch-depth: 0 + + - name: Install uv + id: uv + if: ${{ !cancelled() && steps.checkout.outcome == 'success' }} + uses: astral-sh/setup-uv@68559e99cc807c1eb7e9238fd43a15bba460eec2 # v10.3.3 + + - name: Set up Python + id: python + if: ${{ !cancelled() && steps.checkout.outcome == 'success' }} + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.14" + + # Explicit prerequisites let independent checks finish after a failure + # without masking that failure or continuing after cancellation. + - name: Run ruff check + if: ${{ !cancelled() && steps.checkout.outcome == 'success' && steps.uv.outcome == 'success' && steps.python.outcome == 'success' }} + run: uvx ruff@0.15.0 check src tests - name: Run git diff --check + if: ${{ !cancelled() && steps.checkout.outcome == 'success' }} shell: bash env: EVENT_NAME: ${{ github.event_name }} @@ -42,12 +67,14 @@ jobs: set -euo pipefail if [ "$EVENT_NAME" = "pull_request" ]; then - git fetch --no-tags --depth=1 origin "+${PR_BASE_SHA}:refs/checks/pr-base" + git fetch --no-tags origin "+${PR_BASE_SHA}:refs/checks/pr-base" git diff --check refs/checks/pr-base HEAD + elif [ "$EVENT_NAME" = "workflow_dispatch" ]; then + git diff-tree --check --no-commit-id --root --diff-merges=first-parent -r "$GITHUB_SHA" elif [ "$PUSH_BEFORE_SHA" = "0000000000000000000000000000000000000000" ]; then git diff-tree --check --no-commit-id --root -r "$GITHUB_SHA" else - git fetch --no-tags --depth=1 origin "+${PUSH_BEFORE_SHA}:refs/checks/push-before" + git fetch --no-tags origin "+${PUSH_BEFORE_SHA}:refs/checks/push-before" git diff --check refs/checks/push-before HEAD fi @@ -57,6 +84,8 @@ jobs: # documentation pass never reformats them. Add new documentation # paths to the DOC_GLOBS list above. - name: Verify the documentation globs match files + id: documentation-globs + if: ${{ !cancelled() && steps.checkout.outcome == 'success' }} shell: bash run: | set -euo pipefail @@ -88,20 +117,16 @@ jobs: fi - name: Run markdownlint-cli2 + if: ${{ !cancelled() && steps.checkout.outcome == 'success' && steps.documentation-globs.outcome == 'success' }} uses: DavidAnson/markdownlint-cli2-action@696eb3f2509270d7b882d2aa68347bc676e7af34 # v25.0.0 with: globs: ${{ env.DOC_GLOBS }} - shellcheck: - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - # shellcheck is preinstalled on ubuntu-latest runners. # Start at --severity=error to block real bugs without flagging style # (notably SC2155). Tighten in a follow-up after cleanup. - name: Run shellcheck on shell scripts + if: ${{ !cancelled() && steps.checkout.outcome == 'success' }} run: git ls-files -z -- '*.sh' | xargs -0 shellcheck --severity=error # macOS ships bash 3.2, where bash 4+ case-modification parameter @@ -109,6 +134,7 @@ jobs: # from the shebang and cannot flag these, so guard explicitly; use tr # for portable case conversion. - name: Reject bash 4+ case-modification expansions + if: ${{ !cancelled() && steps.checkout.outcome == 'success' }} run: | matches=$(git ls-files -z -- '*.sh' | xargs -0 grep -nE '\$\{[A-Za-z_][A-Za-z0-9_]*(\[[^]]*\])?(\^\^?|,,?|~~?|@[UuLl])[^}]*\}' || true) if [ -n "$matches" ]; then @@ -116,3 +142,45 @@ jobs: echo "$matches" exit 1 fi + + - name: Check committed audit requirements are current + if: ${{ !cancelled() && steps.checkout.outcome == 'success' && steps.uv.outcome == 'success' && steps.python.outcome == 'success' }} + env: + DEPENDENCY_DIFF_BASE: ${{ github.event.pull_request.base.sha || github.event.before || '' }} + DEPENDENCY_DIFF_HEAD: ${{ github.event.pull_request.head.sha || github.sha }} + GENERATED_REQUIREMENTS: ${{ runner.temp }}/security-audit-requirements.txt + run: python .github/scripts/check_security_requirements.py + + - name: Run pip-audit (committed requirements) + if: ${{ !cancelled() && steps.checkout.outcome == 'success' && steps.uv.outcome == 'success' && steps.python.outcome == 'success' }} + run: uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r .github/security-audit-requirements.txt --progress-spinner off + + dependency-audit-scheduled: + name: Dependency audit scheduled (${{ matrix.os }}, Python ${{ matrix.python-version }}) + if: ${{ github.event_name == 'schedule' }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + python-version: ["3.11", "3.12", "3.13", "3.14", "3.15"] + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Install uv + uses: astral-sh/setup-uv@68559e99cc807c1eb7e9238fd43a15bba460eec2 # v10.3.3 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: ${{ matrix.python-version }} + + # Keep live per-platform resolution separate from the deterministic + # committed snapshot used by push, pull request, and manual runs. + - name: Compile scheduled audit requirements + run: | + uv pip compile pyproject.toml --extra test --python-version "${{ matrix.python-version }}" --upgrade --generate-hashes --quiet --output-file "${{ runner.temp }}/spec-kit-audit-requirements.txt" + + - name: Run pip-audit (scheduled live resolution) + run: uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r "${{ runner.temp }}/spec-kit-audit-requirements.txt" --progress-spinner off diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml deleted file mode 100644 index 90035217fb..0000000000 --- a/.github/workflows/security.yml +++ /dev/null @@ -1,78 +0,0 @@ -name: Security Audit - -permissions: - contents: read - -on: - push: - branches: ["main"] - pull_request: - types: [opened, synchronize, reopened] - schedule: - - cron: "17 4 * * 1" - workflow_dispatch: - -jobs: - dependency-audit: - name: Dependency audit - if: ${{ github.event_name != 'schedule' }} - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - fetch-depth: 0 - - - name: Install uv - uses: astral-sh/setup-uv@68559e99cc807c1eb7e9238fd43a15bba460eec2 # v10.3.3 - - - name: Set up Python - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - with: - python-version: "3.14" - - - name: Check committed audit requirements are current - env: - DEPENDENCY_DIFF_BASE: ${{ github.event.pull_request.base.sha || github.event.before || '' }} - DEPENDENCY_DIFF_HEAD: ${{ github.event.pull_request.head.sha || github.sha }} - GENERATED_REQUIREMENTS: ${{ runner.temp }}/security-audit-requirements.txt - run: python .github/scripts/check_security_requirements.py - - - name: Run pip-audit (committed requirements) - run: uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r .github/security-audit-requirements.txt --progress-spinner off - - dependency-audit-scheduled: - name: Dependency audit scheduled (${{ matrix.os }}, Python ${{ matrix.python-version }}) - if: ${{ github.event_name == 'schedule' }} - runs-on: ${{ matrix.os }} - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, windows-latest] - python-version: ["3.11", "3.12", "3.13", "3.14", "3.15"] - steps: - - name: Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - - name: Install uv - uses: astral-sh/setup-uv@68559e99cc807c1eb7e9238fd43a15bba460eec2 # v10.3.3 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - with: - python-version: ${{ matrix.python-version }} - - # The committed .github/security-audit-requirements.txt is generated with - # --universal (resolves across all interpreters/platforms) and is what - # push/PR/workflow_dispatch runs audit. The scheduled job instead compiles - # per matrix entry with --python-version so it can surface advisories in - # wheels that only resolve on a specific interpreter (e.g. 3.11-only) — - # coverage the universal file may not exercise. This broadening is - # intentional; non-scheduled runs trade that depth for determinism against - # the committed snapshot. - - name: Compile scheduled audit requirements - run: | - uv pip compile pyproject.toml --extra test --python-version "${{ matrix.python-version }}" --upgrade --generate-hashes --quiet --output-file "${{ runner.temp }}/spec-kit-audit-requirements.txt" - - - name: Run pip-audit (scheduled live resolution) - run: uvx --from pip-audit==2.10.0 pip-audit --disable-pip --require-hashes -r "${{ runner.temp }}/spec-kit-audit-requirements.txt" --progress-spinner off diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 097de4a559..93b81c255f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,4 +1,4 @@ -name: Test & Lint Python +name: Test Python permissions: contents: read @@ -12,23 +12,6 @@ on: - cron: "17 4 * * 1" jobs: - ruff: - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - - name: Install uv - uses: astral-sh/setup-uv@68559e99cc807c1eb7e9238fd43a15bba460eec2 # v10.3.3 - - - name: Set up Python - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - with: - python-version: "3.14" - - - name: Run ruff check - run: uvx ruff@0.15.0 check src tests - pytest: runs-on: ${{ matrix.os }} strategy: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2e7fabe9f1..2a830cc482 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -313,18 +313,23 @@ uv pip compile pyproject.toml --extra test --universal --upgrade --generate-hash The scheduled CI audit resolves the runtime and `test` extra dependency set across the supported Python and OS matrix to catch newly published advisories. Upstream package releases drift over time, so even an unrelated PR touching -`pyproject.toml` can fail the `dependency-audit` check until the committed file +`pyproject.toml` can fail the `Lint & dependency audit` check until the committed file is regenerated with the command above and re-committed. +The `lint.yml` workflow runs Ruff, documentation lint, shell checks, and the +committed dependency audit in one job with shared checkout and tool setup. +Independent checks still run after another check fails, and any failure fails +the job. The scheduled dependency-audit matrix runs separately. + #### Shell scripts ```bash git ls-files -z -- '*.sh' | xargs -0 shellcheck --severity=error ``` -The CI `lint.yml` `shellcheck` job currently reports and blocks only +The CI `lint.yml` ShellCheck step currently reports and blocks only error-severity findings. Warnings such as SC2155 are intentionally outside this -job until a follow-up cleanup tightens the threshold. +check until a follow-up cleanup tightens the threshold. ### Manual testing diff --git a/tests/test_github_workflows.py b/tests/test_github_workflows.py index 11262597a0..a45d09f527 100644 --- a/tests/test_github_workflows.py +++ b/tests/test_github_workflows.py @@ -2286,7 +2286,7 @@ def test_bug_test_install_preserves_editable_extras_and_failure(compiled_step, e def _markdownlint_job() -> dict: workflow = yaml.safe_load(LINT_WORKFLOW.read_text(encoding="utf-8")) - return workflow["jobs"]["markdownlint"] + return workflow["jobs"]["checks"] def _documentation_globs() -> list[str]: @@ -2321,3 +2321,78 @@ def test_markdownlint_step_lints_the_documentation_globs(): ] assert len(linters) == 1 assert linters[0]["with"]["globs"] == "${{ env.DOC_GLOBS }}" + + +@requires_bash +@pytest.mark.parametrize("event", [ + "pull_request", "push", "initial_push", "workflow_dispatch", "manual_root", "manual_merge", +]) +@pytest.mark.parametrize("contents,exit_code", [("line\n", 0), ("line \n", 2)]) +def test_combined_lint_run_checks_commit_whitespace(tmp_path, event, contents, exit_code): + env = os.environ.copy() + env.update({ + "GIT_AUTHOR_NAME": "Test", + "GIT_AUTHOR_EMAIL": "test@example.com", + "GIT_COMMITTER_NAME": "Test", + "GIT_COMMITTER_EMAIL": "test@example.com", + "EVENT_NAME": ( + "workflow_dispatch" if event.startswith("manual_") + else "push" if event == "initial_push" else event + ), + "PR_BASE_SHA": "", + "PUSH_BEFORE_SHA": "", + "GITHUB_SHA": "HEAD", + }) + subprocess.run(["git", "init", "--quiet"], cwd=tmp_path, env=env, check=True) + if event not in {"initial_push", "manual_root"}: + (tmp_path / "example.txt").write_text("original\n", encoding="utf-8") + subprocess.run(["git", "add", "example.txt"], cwd=tmp_path, env=env, check=True) + subprocess.run( + ["git", "-c", "commit.gpgsign=false", "commit", "--quiet", "-m", "Base"], + cwd=tmp_path, env=env, check=True, + ) + base = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=tmp_path, env=env, + capture_output=True, text=True, check=True, + ).stdout.strip() + env["PR_BASE_SHA"] = base + env["PUSH_BEFORE_SHA"] = base + if event == "manual_merge": + subprocess.run( + ["git", "checkout", "--quiet", "-b", "whitespace-change"], + cwd=tmp_path, env=env, check=True, + ) + else: + env["PUSH_BEFORE_SHA"] = "0" * 40 + (tmp_path / "example.txt").write_text(contents, encoding="utf-8") + subprocess.run(["git", "add", "example.txt"], cwd=tmp_path, env=env, check=True) + subprocess.run( + ["git", "-c", "commit.gpgsign=false", "commit", "--quiet", "-m", "Change"], + cwd=tmp_path, env=env, check=True, + ) + if event == "manual_merge": + subprocess.run( + ["git", "checkout", "--quiet", "--detach", base], + cwd=tmp_path, env=env, check=True, + ) + subprocess.run( + ["git", "-c", "commit.gpgsign=false", "merge", "--quiet", "--no-ff", + "-m", "Merge change", "whitespace-change"], + cwd=tmp_path, env=env, check=True, + ) + parents = subprocess.run( + ["git", "rev-list", "--parents", "-n", "1", "HEAD"], + cwd=tmp_path, env=env, capture_output=True, text=True, check=True, + ).stdout.split() + assert len(parents) == 3 + subprocess.run( + ["git", "remote", "add", "origin", str(tmp_path)], + cwd=tmp_path, env=env, check=True, + ) + step = _workflow_step(_markdownlint_job()["steps"], "Run git diff --check") + result = subprocess.run( + ["bash", "-euo", "pipefail", "-c", step["run"]], + cwd=tmp_path, env=env, capture_output=True, text=True, check=False, + ) + assert result.returncode == exit_code, result.stderr + assert ("trailing whitespace" in result.stdout) is (exit_code != 0) diff --git a/tests/test_security_workflow.py b/tests/test_security_workflow.py index fc5959d29b..656acae4a5 100644 --- a/tests/test_security_workflow.py +++ b/tests/test_security_workflow.py @@ -7,11 +7,12 @@ import subprocess from pathlib import Path +import pytest import yaml REPO_ROOT = Path(__file__).resolve().parent.parent -SECURITY_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "security.yml" +SECURITY_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "lint.yml" CONTRIBUTING = REPO_ROOT / "CONTRIBUTING.md" SECURITY_REQUIREMENTS = REPO_ROOT / ".github" / "security-audit-requirements.txt" SECURITY_REQUIREMENTS_SYNC_SCRIPT = ( @@ -82,15 +83,102 @@ def _load_sync_script(): return module +CHECK_STEPS = ( + "Run ruff check", + "Run git diff --check", + "Verify the documentation globs match files", + "Run markdownlint-cli2", + "Run shellcheck on shell scripts", + "Reject bash 4+ case-modification expansions", + "Check committed audit requirements are current", + "Run pip-audit (committed requirements)", +) +PYTHON_CHECK_STEPS = ( + "Run ruff check", + "Check committed audit requirements are current", + "Run pip-audit (committed requirements)", +) + + +def _check_runs(step_name, outcomes, cancelled=False): + condition = _step("checks", step_name)["if"] + assert condition.startswith("${{ ") and condition.endswith(" }}") + terms = condition[4:-3].split(" && ") + assert terms[0] == "!cancelled()" + allowed = not cancelled + for term in terms[1:]: + match = re.fullmatch(r"steps\.([\w-]+)\.outcome == 'success'", term) + assert match is not None, f"Unexpected prerequisite: {term}" + allowed = allowed and outcomes.get(match[1]) == "success" + return allowed + + +class TestCombinedChecks: + def test_checks_share_one_job_and_tool_setup(self): + workflow = _load_security_workflow() + assert set(workflow["jobs"]) == {"checks", "dependency-audit-scheduled"} + job = workflow["jobs"]["checks"] + assert job["name"] == "Lint & dependency audit" + assert job.get("continue-on-error", False) is False + assert workflow["permissions"] == {"contents": "read"} + steps = job["steps"] + for action in ("actions/checkout@", "actions/setup-python@", "astral-sh/setup-uv@"): + assert sum(step.get("uses", "").startswith(action) for step in steps) == 1 + assert _step("checks", "Run ruff check")["run"] == ( + "uvx ruff@0.15.0 check src tests" + ) + test_workflow = yaml.safe_load( + (REPO_ROOT / ".github/workflows/test.yml").read_text(encoding="utf-8") + ) + assert set(test_workflow["jobs"]) == {"pytest"} + assert not (REPO_ROOT / ".github/workflows/security.yml").exists() + for step in steps: + assert step.get("continue-on-error", False) is False + assert "--depth" not in _step("checks", "Run git diff --check")["run"] + + @pytest.mark.parametrize("failed_step", [None, *CHECK_STEPS]) + def test_independent_checks_run_even_after_a_check_fails(self, failed_step): + outcomes = dict.fromkeys( + ("checkout", "uv", "python", "documentation-globs"), "success" + ) + if failed_step == "Verify the documentation globs match files": + outcomes["documentation-globs"] = "failure" + elif failed_step is not None: + step_id = _step("checks", failed_step).get("id", failed_step) + outcomes[step_id] = "failure" + for step_name in CHECK_STEPS: + expected = not ( + step_name == "Run markdownlint-cli2" + and failed_step == "Verify the documentation globs match files" + ) + assert _check_runs(step_name, outcomes) is expected + # A failed check is not a prerequisite of another independent check. + assert all( + not _check_runs(step_name, outcomes, cancelled=True) + for step_name in CHECK_STEPS + ) + + @pytest.mark.parametrize("prerequisite", ["checkout", "uv", "python"]) + @pytest.mark.parametrize("outcome", ["failure", "skipped", "cancelled"]) + def test_failed_setup_only_blocks_checks_that_require_it(self, prerequisite, outcome): + outcomes = dict.fromkeys( + ("checkout", "uv", "python", "documentation-globs"), "success" + ) + outcomes[prerequisite] = outcome + for step_name in CHECK_STEPS: + expected = prerequisite != "checkout" and step_name not in PYTHON_CHECK_STEPS + assert _check_runs(step_name, outcomes) is expected + + class TestDependencyAuditWorkflow: """Guard the dependency-audit security workflow.""" def test_dependency_audit_uses_committed_requirements_for_prs_and_pushes(self): workflow = _load_security_workflow() - job = workflow["jobs"]["dependency-audit"] - committed_audit = _step("dependency-audit", "Run pip-audit (committed requirements)") - sync_check = _step("dependency-audit", "Check committed audit requirements are current") - setup_python = _step("dependency-audit", "Set up Python") + job = workflow["jobs"]["checks"] + committed_audit = _step("checks", "Run pip-audit (committed requirements)") + sync_check = _step("checks", "Check committed audit requirements are current") + setup_python = _step("checks", "Set up Python") assert job["if"] == "${{ github.event_name != 'schedule' }}" assert job["runs-on"] == "ubuntu-latest" @@ -106,7 +194,7 @@ def test_dependency_audit_uses_committed_requirements_for_prs_and_pushes(self): assert committed_audit["run"] == LOCAL_PIP_AUDIT dependency_job_text = _job_run_text( - "dependency-audit", + "checks", "dependency-audit-scheduled", ) protection_text = ( @@ -127,7 +215,7 @@ def test_dependency_audit_uses_committed_requirements_for_prs_and_pushes(self): assert "/tmp/" not in protection_text def test_dependency_audit_checkout_fetches_full_history_for_diff_base(self): - checkout = _step("dependency-audit", "Checkout") + checkout = _step("checks", "Checkout") assert checkout["with"]["fetch-depth"] == 0 @@ -135,11 +223,7 @@ def test_security_workflow_triggers(self): triggers = _workflow_triggers() assert triggers["push"]["branches"] == ["main"] - # Asserted by inclusion so later PRs (e.g. baseline-growth gates) can add - # labeled/unlabeled without rewriting this test. - assert {"opened", "synchronize", "reopened"} <= set( - triggers["pull_request"]["types"] - ) + assert triggers["pull_request"] is None assert "workflow_dispatch" in triggers assert triggers["schedule"] == [{"cron": "17 4 * * 1"}]