From 634764729d265c8e49193fae58a516639964d474 Mon Sep 17 00:00:00 2001 From: Samran Asif Date: Wed, 9 Sep 2026 14:38:44 +0500 Subject: [PATCH] fix: make two green gates actually check something, and close the security-gate gaps **The result-artifact gate validated nothing.** The step named "Validate bundled result artifacts against schemas/result-v1" loaded the schema and called `Draft202012Validator.check_schema(schema)`. That checks the schema *document* is well-formed JSON Schema; it never loaded an artifact. Required, branch-protected, green, and proving nothing about its own name. Running it for real (scripts/validate_result_artifacts.py, six commands against the bundled fixtures) immediately found five disagreements between the schema and the emitters. Two are genuine code defects, not schema pedantry: - `changes` was the array of changes in `diff` and an integer count in `breaking`. Same key, two types, from one tool. - `operations` was an integer count in `validate` and `self-test`, but the array of per-operation stats everywhere else -- and performance/engine.py reads it back as that array (`{o["operation_key"]: o for o in baseline["operations"]}`). Handing it a `validate` artifact would raise. Both counts are renamed to `change_count` / `operation_count`. Nothing consumed the integer forms; every consumer in apiverity, tests and web/ reads the array forms, which are untouched. The other three were the schema being wrong: `seed`, `target` and `operation_key` are legitimately null when they do not apply -- that is the provenance honesty this project fixed earlier -- and `rules`, `plugins`, `changelog` and `self-test` emit artifacts but were missing from the command enum. **Pinned actions lied about their versions.** Five `uses:` lines pinned actions/setup-python@5fda3b95a4ea and annotated it `# v5.6.0`. That SHA is v7.0.0 -- two majors newer -- and devrepro-doctor and local-ai-hardware-bench pin the identical SHA while correctly calling it v7.0.0. scripts/check_action_pins.py resolves every pinned SHA against the GitHub API and fails on a comment that disagrees, or on any action not pinned at all. **Security gates.** This repository ran pip-audit and nothing on the npm side, so half its dependency surface went unexamined under a green "Dependency scan". Its only secret check was a local pre-commit hook that `--no-verify` skips and CI never ran. CodeQL now uses `security-extended`, matching local-ai-hardware-bench. **The secret scanner needed its own test more than anything else here.** Planting a real-shaped secret of every class it claims to detect found that it had no Slack, Google, Stripe or npm rule at all -- four classes silently unchecked -- and that my first placeholder heuristic hid a real-shaped npm token because it ended in `0123456789`. I reached for Shannon entropy first. Measuring it showed the fixture `sk-abcdefghijklmnop1234` scores 4.44, *higher* than a real GitHub PAT at 4.14, because a sequential alphabet maximises character diversity. Entropy was the wrong instrument and the measurement is what said so; the replacement requires an ascending run to dominate the value rather than merely appear in it. GitHub's push protection then rejected the first version of the test file, reading the Slack fixture as a live token. That is correct on its side and useful evidence on ours -- the fixtures are realistic enough for a real scanner to bite. It offered an "allow this secret" URL; taking it would teach this repository to wave detections away. The fixtures are assembled from fragments at runtime instead, so the test still exercises fully-formed values while no complete credential-shaped literal exists in the tree for anyone's scanner to trip over. That also shrank the explicit-exemption count from twelve lines to one. Verified: ruff, ruff format, mypy (90 files), 428 tests pass, coverage 81.12% against the 72% floor, and all six repeatable gates clean (e2e, rule catalog, README capture, result artifacts, secret scan, action pins). --- .github/workflows/api-verity.yml | 2 +- .github/workflows/ci.yml | 43 +++- .github/workflows/codeql.yml | 6 + .github/workflows/release.yml | 2 +- apiverity/cli/commands/governance.py | 18 +- apiverity/cli/commands/platform.py | 4 +- schemas/result-v1.schema.json | 155 ++++++++++--- scripts/check_action_pins.py | 159 +++++++++++++ scripts/secret_scan.py | 209 ++++++++++++++++++ scripts/validate_result_artifacts.py | 141 ++++++++++++ .../unit/test_secret_scan_catches_secrets.py | 145 ++++++++++++ 11 files changed, 843 insertions(+), 41 deletions(-) create mode 100644 scripts/check_action_pins.py create mode 100644 scripts/secret_scan.py create mode 100644 scripts/validate_result_artifacts.py create mode 100644 tests/unit/test_secret_scan_catches_secrets.py diff --git a/.github/workflows/api-verity.yml b/.github/workflows/api-verity.yml index 1097e61..512f284 100644 --- a/.github/workflows/api-verity.yml +++ b/.github/workflows/api-verity.yml @@ -31,7 +31,7 @@ jobs: with: fetch-depth: 0 - - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v5.6.0 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" - run: pip install -e . diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1d187ac..3d4b4e6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v5.6.0 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" cache: pip @@ -54,6 +54,13 @@ jobs: # fails if the committed README stops matching what the code produces. run: python scripts/capture_readme_examples.py --check + - name: Secret scan + # The only secret check here was a local pre-commit `detect-private-key` + # hook, which `git commit --no-verify` skips and CI never ran. This is + # the scanner tooltrace-bench uses, plus the Slack, Google, Stripe and + # npm rules a planted-secret test showed it was missing. + run: python scripts/secret_scan.py + # pyproject declares "Operating System :: OS Independent" and Python 3.11 and # 3.12, but CI ran ubuntu-latest on 3.12 only -- so a 3.11 user, or anyone on # Windows or macOS, was running wholly unvalidated code against a classifier @@ -82,7 +89,7 @@ jobs: python-version: "3.12" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v5.6.0 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: ${{ matrix.python-version }} cache: pip @@ -100,19 +107,27 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v5.6.0 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" - run: pip install jsonschema pyyaml && pip install -e . - name: Validate bundled result artifacts against schemas/result-v1 - run: | - python - <<'PY' - import json, pathlib - from jsonschema import Draft202012Validator - schema = json.loads(pathlib.Path("schemas/result-v1.schema.json").read_text()) - Draft202012Validator.check_schema(schema) - print("result-v1 schema is valid") - PY + # This step used to load the schema, call `check_schema` on it, and + # print "result-v1 schema is valid". That validates the schema + # *document*; it never loaded an artifact. It was green, required and + # branch-protected while proving nothing about its own name. + # + # Running it for real immediately found five disagreements between the + # schema and the emitters, including `changes` and `operations` each + # carrying an integer in one command and an array in another. + run: python scripts/validate_result_artifacts.py + - name: Pinned actions match their version comments + # Five `uses:` lines pinned setup-python@5fda3b95a4ea and called it + # "v5.6.0". That SHA is v7.0.0. The pin is the control; the comment is + # what a reviewer reads. + env: + GITHUB_TOKEN: ${{ github.token }} + run: python scripts/check_action_pins.py frontend: name: Frontend lint · typecheck · build @@ -130,6 +145,12 @@ jobs: - run: npm ci || npm install - run: npm run lint - run: npm test + # local-ai-hardware-bench has audited its frontend dependencies from the + # start; this repository ran pip-audit and nothing on the npm side, so + # half its dependency surface was unexamined while a job named + # "Dependency scan" was green. + - name: npm audit (fail on high+) + run: npm audit --audit-level=high - run: npm run build # Code splitting is invisible when it breaks: a static import of a page # module collapses every chunk back into the entry and nothing fails. diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 91f300c..5f39b4c 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -23,6 +23,12 @@ jobs: - uses: github/codeql-action/init@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 with: languages: ${{ matrix.language }} + # The default suite skips a lot that matters for a tool which parses + # untrusted specs and serves HTTP: it was `security-extended` that + # surfaced the ReDoS in a sibling repository's own guard regex. + # local-ai-hardware-bench has run this suite for a while; the four + # projects should not disagree about how hard they look. + queries: security-extended - uses: github/codeql-action/autobuild@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 - uses: github/codeql-action/analyze@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 with: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9319840..40b76a9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v5.6.0 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" - name: Install build tool diff --git a/apiverity/cli/commands/governance.py b/apiverity/cli/commands/governance.py index ef66d72..c7b683e 100644 --- a/apiverity/cli/commands/governance.py +++ b/apiverity/cli/commands/governance.py @@ -28,7 +28,14 @@ def cmd_validate(args: argparse.Namespace) -> int: "protocol": plugin.protocol().value, "title": service.title, "version": service.version, - "operations": len(service.operations), + # `operations` in a result-v1 artifact is the array of per-operation + # stats that performance/engine.py reads back from a baseline + # (`{o["operation_key"]: o for o in baseline["operations"]}`). Emitting + # an integer under the same key meant a `validate` artifact and a + # `baseline` artifact disagreed about the type of the same field, and + # handing the former to the performance engine would raise rather than + # report. The count is still useful, under a name that says so. + "operation_count": len(service.operations), "findings": all_findings, "errors": errors, } @@ -86,7 +93,14 @@ def cmd_breaking(args: argparse.Namespace) -> int: "command": "breaking", "old_version": old.version, "new_version": new.version, - "changes": len(changes), + # `diff` emits `changes` as the array of changes; this emitted the + # same key as an integer count, so a consumer reading `changes` + # got a list or a number depending on which command produced the + # artifact -- and schemas/result-v1 declares it an array, which + # made every `breaking` artifact silently non-conformant. Renamed + # rather than converted: the count is genuinely useful here, and + # `breaking` reports findings, not the changes themselves. + "change_count": len(changes), "findings": findings, "errors": errors, }, diff --git a/apiverity/cli/commands/platform.py b/apiverity/cli/commands/platform.py index 81f4e14..27442ce 100644 --- a/apiverity/cli/commands/platform.py +++ b/apiverity/cli/commands/platform.py @@ -114,7 +114,9 @@ def cmd_self_test(args: argparse.Namespace) -> int: "tool": "apiverity", "command": "self-test", "ok": ok, - "operations": len(service.operations), + # See governance.py: `operations` is the per-operation array in a + # result-v1 artifact, not a count. + "operation_count": len(service.operations), "spec_findings": len(findings), }, args.json, diff --git a/schemas/result-v1.schema.json b/schemas/result-v1.schema.json index 0baf0b9..86f3da4 100644 --- a/schemas/result-v1.schema.json +++ b/schemas/result-v1.schema.json @@ -3,44 +3,149 @@ "$id": "https://github.com/webdevsamran/api-verity-lab/schemas/result-v1.schema.json", "title": "apiverity result artifact (v1)", "type": "object", - "required": ["tool", "tool_version", "command", "contract_hash"], + "required": [ + "tool", + "tool_version", + "command", + "contract_hash" + ], "properties": { - "tool": { "const": "apiverity" }, - "tool_version": { "type": "string" }, - "result_schema_version": { "const": 1 }, + "tool": { + "const": "apiverity" + }, + "tool_version": { + "type": "string" + }, + "result_schema_version": { + "const": 1 + }, "command": { - "enum": ["validate", "diff", "breaking", "test", "workflow", "drift", - "replay", "baseline", "regression", "coverage"] - }, - "contract_hash": { "type": "string", "pattern": "^[a-f0-9]{64}$" }, - "protocol": { "enum": ["openapi", "graphql", "grpc"] }, - "target": { "type": "string" }, - "seed": { "type": "integer" }, - "duration_ms": { "type": "integer" }, + "enum": [ + "baseline", + "breaking", + "changelog", + "coverage", + "diff", + "drift", + "plugins", + "regression", + "replay", + "rules", + "self-test", + "test", + "validate", + "workflow" + ] + }, + "contract_hash": { + "type": "string", + "pattern": "^[a-f0-9]{64}$" + }, + "protocol": { + "enum": [ + "openapi", + "graphql", + "grpc" + ] + }, + "target": { + "type": [ + "string", + "null" + ], + "description": "Base URL contacted, or null when nothing was contacted." + }, + "seed": { + "type": [ + "integer", + "null" + ], + "description": "Generation seed, or null when the command is not seeded." + }, + "duration_ms": { + "type": "integer" + }, "redaction": { "type": "object", "properties": { - "applied": { "type": "boolean" }, - "sensitive_field_count": { "type": "integer" } + "applied": { + "type": "boolean" + }, + "sensitive_field_count": { + "type": "integer" + } } }, "findings": { "type": "array", "items": { "type": "object", - "required": ["rule_id", "severity", "message"], + "required": [ + "rule_id", + "severity", + "message" + ], "properties": { - "rule_id": { "type": "string" }, - "severity": { "enum": ["ERROR", "WARN", "INFO"] }, - "message": { "type": "string" }, - "operation_key": { "type": "string" }, - "old_location": { "type": "object" }, - "new_location": { "type": "object" } + "rule_id": { + "type": "string" + }, + "severity": { + "enum": [ + "ERROR", + "WARN", + "INFO" + ] + }, + "message": { + "type": "string" + }, + "operation_key": { + "type": [ + "string", + "null" + ], + "description": "Operation the finding belongs to, or null for contract-level findings." + }, + "old_location": { + "type": [ + "object", + "null" + ] + }, + "new_location": { + "type": [ + "object", + "null" + ] + } } } }, - "changes": { "type": "array", "items": { "type": "object" } }, - "results": { "type": "array", "items": { "type": "object" } }, - "operations": { "type": "array", "items": { "type": "object" } } + "changes": { + "type": "array", + "items": { + "type": "object" + } + }, + "results": { + "type": "array", + "items": { + "type": "object" + } + }, + "operations": { + "type": "array", + "items": { + "type": "object" + } + }, + "change_count": { + "type": "integer", + "description": "Number of changes considered (breaking)." + }, + "operation_count": { + "type": "integer", + "description": "Number of operations in the contract (validate, self-test)." + } } -} \ No newline at end of file +} diff --git a/scripts/check_action_pins.py b/scripts/check_action_pins.py new file mode 100644 index 0000000..78dd585 --- /dev/null +++ b/scripts/check_action_pins.py @@ -0,0 +1,159 @@ +"""Every pinned GitHub Action must be pinned, and its version comment must be true. + +Pinning an action to a commit SHA is the supply-chain control; the trailing +`# vX.Y.Z` comment is what a human reviewer actually reads. When the two +disagree, the comment wins in the reviewer's head and the control is worth +less than it looks. + +That is not hypothetical here. Five `uses:` lines in this repository pinned +`actions/setup-python@5fda3b95a4ea` and annotated it `# v5.6.0`. That SHA is +**v7.0.0** -- two majors newer -- and two sibling repositories pinned the exact +same SHA while correctly calling it v7.0.0. Anyone auditing this repo's +supply chain would have concluded it was running a version it was not. + +Two rules are enforced: + +1. Every third-party `uses:` is pinned to a full 40-character SHA. A floating + tag (`@v4`) means the action's content can change under you. +2. Where a pin carries a version comment, some tag at that SHA matches it. + +Resolving a SHA to its tags needs the network, so this runs in CI rather than +in the unit-test suite. `GITHUB_TOKEN` lifts the rate limit from 60/hour to +5000; results are cached per (action, sha), so a full run is a handful of +calls. + + python scripts/check_action_pins.py +""" + +from __future__ import annotations + +import json +import os +import re +import sys +import urllib.error +import urllib.request +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +WORKFLOWS = ROOT / ".github" / "workflows" + +#: `uses: owner/repo[/subpath]@` with an optional trailing `# comment`. +USES = re.compile( + r"^\s*-?\s*uses:\s*" + r"(?P[\w.-]+/[\w.-]+)" + r"(?P(?:/[\w.-]+)*)" + r"@(?P[\w.\-/]+)" + r"(?:\s*#\s*(?P.*?))?\s*$" +) +VERSION_IN_COMMENT = re.compile(r"\bv(\d+(?:\.\d+)*)\b") +FULL_SHA = re.compile(r"^[0-9a-f]{40}$") + +#: Local composite actions (`./.github/actions/...`) have no upstream to pin. +LOCAL = re.compile(r"^\s*-?\s*uses:\s*\.") + +_cache: dict[tuple[str, str], list[str]] = {} + + +def tags_at(action: str, sha: str) -> list[str]: + """Tag names pointing at `sha` in `action`'s repository.""" + key = (action, sha) + if key in _cache: + return _cache[key] + + request = urllib.request.Request( + f"https://api.github.com/repos/{action}/tags?per_page=100", + headers={ + "Accept": "application/vnd.github+json", + "User-Agent": "check-action-pins", + }, + ) + token = os.environ.get("GITHUB_TOKEN") or os.environ.get("GH_TOKEN") + if token: + request.add_header("Authorization", f"Bearer {token}") + + try: + with urllib.request.urlopen(request, timeout=30) as response: + payload = json.load(response) + except (urllib.error.URLError, TimeoutError, json.JSONDecodeError) as exc: + print(f"warning: could not resolve tags for {action}: {exc}", file=sys.stderr) + _cache[key] = [] + return [] + + tags = [t["name"] for t in payload if t.get("commit", {}).get("sha") == sha] + _cache[key] = tags + return tags + + +def main() -> int: + if not WORKFLOWS.is_dir(): + print("no .github/workflows directory", file=sys.stderr) + return 1 + + unpinned: list[str] = [] + mislabelled: list[str] = [] + unresolved: list[str] = [] + checked = 0 + + for workflow in sorted(WORKFLOWS.glob("*.yml")) + sorted(WORKFLOWS.glob("*.yaml")): + for lineno, line in enumerate(workflow.read_text(encoding="utf-8").splitlines(), 1): + if LOCAL.match(line): + continue + match = USES.match(line) + if not match: + continue + + action = match.group("action") + ref = match.group("ref") + where = f"{workflow.name}:{lineno}" + + if not FULL_SHA.match(ref): + unpinned.append(f"{where} {action}@{ref} is not pinned to a full SHA") + continue + + claimed = VERSION_IN_COMMENT.search(match.group("comment") or "") + if not claimed: + continue + + checked += 1 + want = "v" + claimed.group(1) + tags = tags_at(action, ref) + if not tags: + unresolved.append( + f"{where} {action}@{ref[:12]} claims {want}; no tag points at that SHA" + ) + elif not any(tag == want or tag.startswith(want + ".") for tag in tags): + mislabelled.append( + f"{where} {action}@{ref[:12]} claims {want}; real tags: {', '.join(tags)}" + ) + + for label, rows in ( + ("actions that are not SHA-pinned", unpinned), + ("pins whose version comment is wrong", mislabelled), + ("pins naming a version no tag confirms", unresolved), + ): + if rows: + print(f"{label}:", file=sys.stderr) + for row in rows: + print(" " + row, file=sys.stderr) + print(file=sys.stderr) + + if unpinned or mislabelled: + print( + "Fix the comment to match the SHA, or repin the SHA to match the comment.", + file=sys.stderr, + ) + return 1 + + if unresolved: + # A tag can legitimately disappear or move; that is worth printing but + # is not grounds to fail a build on someone else's repository state. + print(f"ok (with {len(unresolved)} unresolved) - {checked} pins checked") + return 0 + + print(f"ok {checked} pinned actions, every version comment verified") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/secret_scan.py b/scripts/secret_scan.py new file mode 100644 index 0000000..dda640c --- /dev/null +++ b/scripts/secret_scan.py @@ -0,0 +1,209 @@ +"""Publication gate: fail if likely secrets appear in tracked files. + +Scans the repository (excluding build/, dist/, web/node_modules, .git) for +high-confidence secret patterns: API keys, bearer tokens, AWS keys, +private keys, generic credential assignments. Exit 1 on any finding. +""" + +from __future__ import annotations + +import itertools +import re +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +SKIP_DIRS = {".git", "node_modules", ".venv", "__pycache__", "build", "dist", "htmlcov"} +SKIP_SUFFIXES = {".lock", ".png", ".jpg", ".ico", ".woff2"} + +PATTERNS: list[tuple[str, re.Pattern[str]]] = [ + ("aws-access-key", re.compile(r"AKIA[0-9A-Z]{16}")), + ("private-key-block", re.compile(r"-----BEGIN [A-Z ]*PRIVATE KEY-----")), + ("bearer-token", re.compile(r"Bearer\s+[A-Za-z0-9\-_.~+/]{32,}")), + ("openai-style-key", re.compile(r"sk-[A-Za-z0-9]{20,}")), + ("github-pat", re.compile(r"gh[pousr]_[A-Za-z0-9]{36,}")), + # Added after a planted-secret test walked a real-shaped Slack bot token + # straight past the scanner: the original pattern list had no Slack, Google, + # Stripe or npm rule at all, so those four classes were never checked. + ("slack-token", re.compile(r"xox[baprs]-[0-9A-Za-z-]{10,}")), + ("google-api-key", re.compile(r"AIza[0-9A-Za-z_\-]{35}")), + ("stripe-live-key", re.compile(r"sk_live_[0-9A-Za-z]{16,}")), + ("npm-token", re.compile(r"npm_[0-9A-Za-z]{36}")), + ( + "credential-assignment", + re.compile( + r"""(?i)(api[_-]?key|secret|password|passwd|token)\s*[:=]\s*""" + r"""["'][^"']{12,}["']""" + ), + ), +] + +ALLOW_SUBSTRINGS = [ + "example", + "placeholder", + "your-api-key", + "", + "dummy", + "test-key", + "xxxx", + "redacted", + "${", + "{{", +] + +#: Words that make up obviously-synthetic fixture credentials. A real token is +#: not spelled out of dictionary words. +_FILLER_WORDS = { + "a", + "acme", + "alice", + "an", + "api", + "auth", + "bearer", + "bob", + "credential", + "creds", + "demo", + "fake", + "foo", + "bar", + "baz", + "here", + "key", + "local", + "mock", + "my", + "name", + "owner", + "pass", + "password", + "sample", + "secret", + "some", + "stub", + "test", + "testing", + "token", + "user", + "value", + "admin", +} + +_SEQUENTIAL_RUN = 6 + + +#: A run must also make up this share of the value before the value is called +#: synthetic. Presence alone is not enough -- a planted-secret test showed a +#: real-shaped npm token (`npm_...WxYz0123456789`) being suppressed purely +#: because it ended in a decimal run. Requiring the run to dominate keeps +#: `sk-abcdefghijklmnop1234` (16 of 23 characters) synthetic while leaving that +#: token detected. +_SEQUENTIAL_SHARE = 0.4 + + +def _longest_sequential_run(value: str) -> int: + longest = run = 1 + for previous, current in itertools.pairwise(value): + run = run + 1 if ord(current) - ord(previous) == 1 else 1 + longest = max(longest, run) + return longest if value else 0 + + +def _has_sequential_run(value: str) -> bool: + """True if an ascending run dominates `value`. + + `sk-abcdefghijklmnop1234` is a fixture, but it is *high* entropy -- a + sequential alphabet maximises character diversity, so it scores 4.44, + above a real GitHub PAT at 4.14. Shannon entropy is the wrong instrument + here, which measuring it is what showed. A dominant consecutive run is + what actually marks a value synthetic. + """ + if len(value) < _SEQUENTIAL_RUN: + return False + longest = _longest_sequential_run(value) + return longest >= _SEQUENTIAL_RUN and longest >= _SEQUENTIAL_SHARE * len(value) + + +def _is_spelled_from_words(value: str) -> bool: + """True if every alphabetic part is a filler word (`owner-token-123`).""" + parts = [p for p in re.split(r"[-_. ]+", value) if p] + alpha = [p for p in parts if p.isalpha()] + if not alpha or len(alpha) < 2: + return False + return all(p.lower() in _FILLER_WORDS for p in alpha) and all( + p.isalpha() or p.isdigit() for p in parts + ) + + +def looks_synthetic(snippet: str) -> bool: + """Whether a matched snippet is a fixture rather than a live credential. + + Deliberately narrow. Both tests describe shapes a generated secret cannot + have, so neither can hide a real key: an ascending run of six characters, + or a value spelled entirely out of dictionary filler words. + """ + quoted = re.findall(r"""["']([^"']{8,})["']""", snippet) + for candidate in [*quoted, snippet]: + if _has_sequential_run(candidate) or _is_spelled_from_words(candidate): + return True + return False + + +#: Marker that exempts a single line. Deliberately verbose so it cannot be +#: typed by accident and is trivial to grep for in review. +ALLOW_MARKER = "secret-scan: allow" + + +def main() -> int: + findings: list[str] = [] + exempted = 0 + for path in ROOT.rglob("*"): + if not path.is_file(): + continue + rel = path.relative_to(ROOT) + if any(part in SKIP_DIRS for part in rel.parts): + continue + if path.suffix.lower() in SKIP_SUFFIXES: + continue + try: + text = path.read_text(encoding="utf-8") + except (UnicodeDecodeError, OSError): + continue + lines = text.splitlines() + for name, pattern in PATTERNS: + for match in pattern.finditer(text): + snippet = match.group(0) + low = snippet.lower() + if any(a in low for a in ALLOW_SUBSTRINGS): + continue + if looks_synthetic(snippet): + continue + line_no = text.count(chr(10), 0, match.start()) + 1 + # Accept the marker on the matched line or either neighbour: + # `ruff format` re-wrapped a call and moved a literal one line + # away from its own marker, which would otherwise reopen a + # finding for a purely cosmetic reformat. + window = lines[max(0, line_no - 2) : line_no + 1] + if any(ALLOW_MARKER in candidate for candidate in window): + # Deliberate, per line, and greppable. Used by the test that + # plants a real-shaped secret of every class to prove this + # scanner still detects them -- that file has to contain the + # very things the scanner looks for. Exempting `tests/` + # wholesale would have been the easy fix and would also hide + # a genuine key committed to a test. + exempted += 1 + continue + findings.append(f"{rel}:{line_no}: {name}") + if findings: + print("LIKELY SECRETS DETECTED — publication blocked:", file=sys.stderr) + for f in findings: + print(f" {f}", file=sys.stderr) + return 1 + suffix = f", {exempted} line(s) exempted by an explicit marker" if exempted else "" + print(f"secret scan clean ({len(list(ROOT.rglob('*')))} paths considered{suffix})") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/validate_result_artifacts.py b/scripts/validate_result_artifacts.py new file mode 100644 index 0000000..d4bfdc1 --- /dev/null +++ b/scripts/validate_result_artifacts.py @@ -0,0 +1,141 @@ +"""Validate real emitted artifacts against `schemas/result-v1.schema.json`. + +The CI step named *"Validate bundled result artifacts against +schemas/result-v1"* did not do that. Its body was: + + schema = json.loads(pathlib.Path("schemas/result-v1.schema.json").read_text()) + Draft202012Validator.check_schema(schema) + print("result-v1 schema is valid") + +`check_schema` checks that the schema *document* is a well-formed JSON Schema. +It never loaded an artifact. So the step was green, required, and +branch-protected while proving nothing about the thing it was named for -- and +a schema change that broke every emitted artifact would have sailed through it. + +This runs the real commands against the bundled fixtures, captures each +`--json` payload, and validates it against the schema. A payload that omits a +required field, or contradicts a declared type, fails the build. + + python scripts/validate_result_artifacts.py +""" + +from __future__ import annotations + +import contextlib +import io +import json +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +SCHEMA_PATH = ROOT / "schemas" / "result-v1.schema.json" +FIXTURES = ROOT / "fixtures" + +sys.path.insert(0, str(ROOT)) + +#: (label, argv). Each is a command a user runs and whose output a CI gate or +#: the frontend consumes, so each must satisfy the published schema. +COMMANDS: list[tuple[str, list[str]]] = [ + ( + "diff", + [ + "diff", + str(FIXTURES / "apis/versioned/v1.yaml"), + str(FIXTURES / "apis/versioned/v2.yaml"), + "--json", + ], + ), + ( + "breaking", + [ + "breaking", + str(FIXTURES / "apis/versioned/v1.yaml"), + str(FIXTURES / "apis/versioned/v2.yaml"), + "--json", + ], + ), + ( + "validate", + ["validate", str(FIXTURES / "apis/versioned/v1.yaml"), "--json"], + ), + ( + "coverage", + ["coverage", str(FIXTURES / "apis/versioned/v1.yaml"), "--json"], + ), + ("rules", ["rules", "--json"]), + ("plugins", ["plugins", "--json"]), +] + + +def run_json(argv: list[str]) -> tuple[int, str]: + """Run the CLI in-process and capture stdout. + + In-process rather than as a subprocess so this needs no installed console + script and reports a real traceback when something breaks. + """ + from apiverity.cli.main import main as cli_main + + buffer = io.StringIO() + code = 0 + try: + with contextlib.redirect_stdout(buffer): + code = cli_main(argv) or 0 + except SystemExit as exc: # argparse and explicit exits + code = exc.code if isinstance(exc.code, int) else 1 + return code, buffer.getvalue() + + +def main() -> int: + try: + from jsonschema import Draft202012Validator + except ImportError: + print("jsonschema is required: pip install jsonschema", file=sys.stderr) + return 1 + + schema = json.loads(SCHEMA_PATH.read_text(encoding="utf-8")) + # Still worth doing -- a malformed schema would make every check below + # vacuously pass, which is how the original step went wrong. + Draft202012Validator.check_schema(schema) + validator = Draft202012Validator(schema) + + failures: list[str] = [] + validated = 0 + + for label, argv in COMMANDS: + code, raw = run_json(argv) + # Findings-bearing commands exit non-zero by contract; that is not a + # failure of the artifact. Only a crash is. + if code not in (0, 1): + failures.append(f"{label}: exited {code}\n{raw[:400]}") + continue + if not raw.strip(): + failures.append(f"{label}: produced no output") + continue + try: + payload = json.loads(raw) + except json.JSONDecodeError as exc: + failures.append(f"{label}: --json output is not JSON: {exc}") + continue + + errors = sorted(validator.iter_errors(payload), key=lambda e: list(e.path)) + if errors: + detail = "; ".join( + f"{'/'.join(str(p) for p in e.path) or ''}: {e.message}" for e in errors[:5] + ) + failures.append(f"{label}: {len(errors)} schema violation(s) -- {detail}") + continue + + validated += 1 + + if failures: + print("result-v1 validation FAILED:", file=sys.stderr) + for failure in failures: + print(" " + failure, file=sys.stderr) + return 1 + + print(f"ok {validated} emitted artifacts validate against result-v1") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/unit/test_secret_scan_catches_secrets.py b/tests/unit/test_secret_scan_catches_secrets.py new file mode 100644 index 0000000..d537445 --- /dev/null +++ b/tests/unit/test_secret_scan_catches_secrets.py @@ -0,0 +1,145 @@ +"""The secret scanner must actually catch secrets. + +A scanner that returns clean is indistinguishable from a scanner that checks +nothing, which is the failure mode this repository has already shipped twice +elsewhere: a CI step named for a validation it never performed, and a schema +gate that validated the schema document instead of any artifact. + +So the scanner is tested the only way that means anything -- by planting a +real-shaped secret of each class it claims to detect and asserting it is +found. Writing this test found that the pattern list had no Slack, Google, +Stripe or npm rule at all, and that a placeholder heuristic was suppressing a +real-shaped npm token because it happened to end in `0123456789`. + +Nothing here is a real credential. Every value is synthetic, and the two the +scanner deliberately ignores are asserted as ignored so the exemptions stay +narrow. +""" + +from __future__ import annotations + +import importlib.util +import re +from pathlib import Path +from typing import Any + +import pytest + +_ROOT = Path(__file__).resolve().parent.parent.parent + + +def _scanner() -> Any: + """Import scripts/secret_scan.py, which is not an importable package.""" + path = _ROOT / "scripts" / "secret_scan.py" + spec = importlib.util.spec_from_file_location("secret_scan", path) + assert spec and spec.loader + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def _matches(module: Any, text: str) -> list[str]: + """Rule names that fire on `text`, after the synthetic-value filter.""" + hits = [] + for name, pattern in module.PATTERNS: + for match in pattern.finditer(text): + snippet = match.group(0) + if any(a in snippet.lower() for a in module.ALLOW_SUBSTRINGS): + continue + if module.looks_synthetic(snippet): + continue + hits.append(name) + return hits + + +#: Synthetic but correctly shaped. Each must trip at least one rule. +#: +#: Assembled from fragments rather than written as whole literals. GitHub's +#: push protection rejected an earlier version of this file -- it read the +#: Slack fixture as a live Slack API token and blocked the push outright. +#: That is the correct behaviour on its side and useful evidence on ours: these +#: values are realistic enough for a real scanner to bite. The offered +#: "allow this secret" URL was the wrong door to walk through; a repository +#: that learns to wave detections away has lost the gate. Joining fragments at +#: runtime keeps the test exercising a fully-formed value while leaving no +#: complete credential-shaped literal in the tree for anyone's scanner -- +#: GitHub's, ours, or a future one -- to trip over. +def _j(*parts: str) -> str: + return "".join(parts) + + +MUST_DETECT = { + "github-pat": 'TOKEN = "' + _j("ghp_", "16C7e42F292c6912E7710c838347Ae178B4a") + '"', + "openai": 'KEY = "' + _j("sk-", "proj9vQx2LmZ8pT4wR7yK1nB3cV6hJ0sD5fG8aE2uI4oP") + '"', + "aws": 'AWS = "' + _j("AKIA", "3KJHF8SDLKJ2HF9X") + '"', + "slack": 'SLACK = "' + _j("xox", "b-2734982374-2938749283-KJHfkjhdsfKJHDSF") + '"', + "google": 'G = "' + _j("AIza", "SyD3kJhGf8sLkJhGf8sLkJhGf8sLkJhGf8s") + '"', + "stripe": 'S = "' + _j("sk_", "live_4eC39HqLyjWDarjtT1zdp7dc") + '"', + "npm": 'N = "' + _j("npm_", "aB3dEfGhIjKlMnOpQrStUvWxYz0123456789") + '"', + "private-key": 'PEM = "' + _j("-----BEGIN ", "RSA PRIVATE KEY-----") + '"', + "password": 'password = "' + _j("Tr0ub4dor", "&3xKcd9zQ") + '"', + # Added because test_every_pattern_is_exercised_by_this_file caught that + # `bearer-token` had only a *suppressed* fixture and no positive case. + "bearer": 'H = "Authorization: ' + + _j("Bearer ", "eyJhbGciOiJIUzI1NiJ9.RkQ7mVn2pXt.9fQ2wZ") + + '"', +} + +#: Fixtures already in this repository that must NOT be reported. Each is a +#: value whose whole purpose is to demonstrate credential *handling*. +MUST_IGNORE = { + "redaction-test-input": '{"Authorization": "' + _j("Bearer ", "sk-abcdefghijklmnop1234") + '"}', + "export-test-token": 'store.add_user(org_id, "alice", "owner", token="secret-token-value")', + "server-fixture-token": 'token = "owner-token-123"', +} + + +@pytest.mark.parametrize("label", sorted(MUST_DETECT)) +def test_a_real_shaped_secret_of_each_class_is_detected(label: str) -> None: + module = _scanner() + hits = _matches(module, MUST_DETECT[label]) + assert hits, ( + f"a real-shaped {label} secret passed the scanner unreported. " + "Either the pattern is missing or a placeholder heuristic is too broad." + ) + + +@pytest.mark.parametrize("label", sorted(MUST_IGNORE)) +def test_the_repositorys_own_fixtures_are_not_reported(label: str) -> None: + module = _scanner() + assert not _matches(module, MUST_IGNORE[label]), ( + f"{label} is a fixture demonstrating credential handling, not a credential; " + "reporting it trains people to ignore this gate" + ) + + +def test_the_synthetic_filter_needs_a_dominant_run_not_merely_a_run() -> None: + """A real token that happens to contain `0123456789` must survive. + + The first version of this heuristic suppressed any value containing a + six-character ascending run, which hid a real-shaped npm token outright. + """ + module = _scanner() + fixture = '"sk-abcdefghijklmnop1234"' # secret-scan: allow + real_token = '"npm_aB3dEfGhIjKlMnOpQrStUvWxYz0123456789"' # secret-scan: allow + assert module.looks_synthetic(fixture) + assert not module.looks_synthetic(real_token) + + +def test_every_pattern_is_exercised_by_this_file() -> None: + """A rule nobody tests is a rule nobody knows works.""" + module = _scanner() + exercised = set() + for text in MUST_DETECT.values(): + exercised.update(_matches(module, text)) + declared = {name for name, _ in module.PATTERNS} + untested = sorted(declared - exercised) + assert not untested, f"these detection rules have no test: {untested}" + + +def test_the_scanner_reports_a_path_and_line() -> None: + """A finding a reader cannot locate is not actionable.""" + source = (_ROOT / "scripts" / "secret_scan.py").read_text(encoding="utf-8") + assert re.search(r"\{rel\}:\{line_no\}", source), ( + "secret_scan.py no longer reports file:line for a finding" + )