From f802294bb99095e2e70989454c6e7a12ed0c763d Mon Sep 17 00:00:00 2001 From: Ishavn Date: Fri, 2 Oct 2026 09:44:35 +0300 Subject: [PATCH 1/2] Fix composite Action installation and multi-file reports --- .github/workflows/ci.yml | 224 ++++++++++++++++++++++++++++++++++++++ CHANGELOG.md | 58 ++++++++++ PROJECT_PLAN.md | 49 +++++++++ README.md | 81 +++++++++++++- TODO.md | 12 ++ action.yml | 17 ++- src/sheetsentry/cli.py | 35 +++++- src/sheetsentry/report.py | 136 ++++++++++++++++++++++- tests/test_cli.py | 154 +++++++++++++++++++++++++- 9 files changed, 751 insertions(+), 15 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 PROJECT_PLAN.md create mode 100644 TODO.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a6d86ad..c69342a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,3 +31,227 @@ jobs: run: | sheetsentry rules sheetsentry audit --help + + action-smoke: + # Runs the actual composite action from a separate checkout of its own + # source in a synthetic consumer workspace, starting without SheetSentry + # installed and without any PyPI package to fall back on. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Check out the action source into its own directory + uses: actions/checkout@v4 + with: + path: action-source + + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Assert SheetSentry is not preinstalled + run: | + if python -m pip show sheetsentry > /dev/null 2>&1; then + echo "sheetsentry is already installed; the smoke test must start without it" + exit 1 + fi + echo "sheetsentry not installed (expected)" + + - name: Generate synthetic workbooks (clean, SD003 duplicate, SD001 text number) + run: | + pip install --quiet openpyxl + mkdir -p consumer "consumer/sub dir" + python - <<'PY' + from openpyxl import Workbook + + wb = Workbook() + ws = wb.active + ws.title = "Data" + ws.append(["id", "value"]) + ws.append([1, "ok"]) + wb.save("consumer/clean.xlsx") + + rows = [[f"A{i}", "item", i] for i in range(1, 25)] + rows[9][0] = "A1" # duplicated SKU -> SD003 error + wb = Workbook() + wb.remove(wb.active) + ws = wb.create_sheet(title="Price") + for row in [["SKU", "Name", "Price"]] + rows: + ws.append(row) + wb.save("consumer/dupes.xlsx") + + rows = [[f"A{i}", "item", i * 10] for i in range(1, 12)] + rows[3][2] = "1 250" # price arrived as text -> SD001 warning + wb = Workbook() + wb.remove(wb.active) + ws = wb.create_sheet(title="Price") + for row in [["SKU", "Name", "Price"]] + rows: + ws.append(row) + wb.save("consumer/sub dir/year end.xlsx") + PY + + - name: Run the composite action with the default recursive pattern (findings expected) + id: audit_recursive + continue-on-error: true + uses: ./action-source + with: + format: sarif + output: consumer/audit.sarif + fail-on: error + + - name: The default invocation must have failed on the SD003 error, not crashed + run: | + echo "outcome: ${STEPS_AUDIT_OUTCOME}, conclusion: ${STEPS_AUDIT_CONCLUSION}" + test "$STEPS_AUDIT_OUTCOME" = "failure" + test "$STEPS_AUDIT_CONCLUSION" = "success" + test -s consumer/audit.sarif + env: + STEPS_AUDIT_OUTCOME: ${{ steps.audit_recursive.outcome }} + STEPS_AUDIT_CONCLUSION: ${{ steps.audit_recursive.conclusion }} + + - name: Run the composite action with a literal path containing spaces + uses: ./action-source + with: + files: consumer/sub dir/year end.xlsx + format: json + output: consumer/literal.json + fail-on: never + + - name: Run the composite action for a JSON envelope + uses: ./action-source + with: + files: consumer/**/*.xlsx + format: json + output: consumer/audit.json + fail-on: never + + - name: Verify installed source metadata and both output documents + run: | + python - <<'PY' + import importlib.metadata as md + import json + import pathlib + + dist = md.distribution("sheetsentry") + direct = json.loads(dist.read_text("direct_url.json") or "{}") + url = direct.get("url", "") + print("version:", dist.version) + print("direct_url:", url) + assert "action-source" in url, f"installed from unexpected source: {url}" + + # SARIF: one document, one run, every file mapped, rules deduplicated. + sarif = json.loads(pathlib.Path("consumer/audit.sarif").read_text()) + assert sarif["version"] == "2.1.0" + assert len(sarif["runs"]) == 1 + run = sarif["runs"][0] + assert run["tool"]["driver"]["name"] == "SheetSentry" + result_uris = { + loc["physicalLocation"]["artifactLocation"]["uri"] + for res in run["results"] for loc in res["locations"] + } + assert "consumer/dupes.xlsx" in result_uris, result_uris + assert "consumer/sub%20dir/year%20end.xlsx" in result_uris, result_uris + rule_ids = {rule["id"] for rule in run["tool"]["driver"]["rules"]} + assert {"SD001", "SD003"} <= rule_ids, rule_ids # SD004 (mixed types) may also fire + artifact_uris = {a["location"]["uri"] for a in run["artifacts"]} + assert artifact_uris == { + "consumer/clean.xlsx", + "consumer/dupes.xlsx", + "consumer/sub%20dir/year%20end.xlsx", + }, artifact_uris + print("SARIF OK: one document,", len(run["results"]), "results,", len(artifact_uris), "artifacts") + + # JSON envelope: one object, every file mapped. + envelope = json.loads(pathlib.Path("consumer/audit.json").read_text()) + assert [r["file"] for r in envelope["reports"]] == [ + "consumer/clean.xlsx", + "consumer/dupes.xlsx", + "consumer/sub dir/year end.xlsx", + ], [r["file"] for r in envelope["reports"]] + assert envelope["errors"] == [] + print("JSON envelope OK:", len(envelope["reports"]), "reports") + + # Literal path with spaces: single-file root shape. + literal = json.loads(pathlib.Path("consumer/literal.json").read_text()) + assert literal["file"] == "consumer/sub dir/year end.xlsx", literal["file"] + assert literal["summary"]["warning"] == 1 + print("Literal spaced path OK:", literal["file"]) + PY + + - name: Review rejects the multi-file JSON envelope with a clear error + run: | + cat > consumer/state.json <<'JSON' + {"version": 1, "items": []} + JSON + set +e + sheetsentry review consumer/audit.json --state consumer/state.json > review-out.txt 2> review-err.txt + code=$? + set -e + cat review-err.txt + test "$code" -eq 2 + grep -q "multi-file envelope" review-err.txt + + - name: Produce single-file and mixed-unreadable SARIF for the schema check + run: | + printf 'not a workbook' > consumer/broken.xlsx + set +e + sheetsentry audit consumer/dupes.xlsx --format sarif -o consumer/single.sarif + single_rc=$? + sheetsentry audit consumer/clean.xlsx consumer/dupes.xlsx consumer/broken.xlsx --format sarif -o consumer/mixed.sarif + mixed_rc=$? + set -e + test "$single_rc" -eq 1 + test "$mixed_rc" -eq 2 + + - name: Validate every SARIF output against the pinned official OASIS schema + run: | + pip install --quiet jsonschema==4.23.0 + python - <<'PY' + import hashlib + import json + import pathlib + import urllib.request + + from jsonschema import Draft4Validator + + SCHEMA_URL = ( + "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/" + "adbb670c018335b0f384e6dd8819f4ea055d7ee1/sarif-2.1/schema/" + "sarif-schema-2.1.0.json" + ) + SCHEMA_SHA256 = "c3b4bb2d6093897483348925aaa73af03b3e3f4bd4ca38cef26dcb4212a2682e" + + schema_path = pathlib.Path("/tmp/sarif-schema-2.1.0.json") + with urllib.request.urlopen(SCHEMA_URL) as response: # TLS verified + schema_path.write_bytes(response.read()) + digest = hashlib.sha256(schema_path.read_bytes()).hexdigest() + print("schema sha256:", digest) + assert digest == SCHEMA_SHA256, f"pinned schema hash mismatch: {digest}" + schema = json.loads(schema_path.read_text()) + validator = Draft4Validator(schema) + + def valid(document_path): + document = json.loads(pathlib.Path(document_path).read_text()) + errors = sorted(validator.iter_errors(document), key=str) + for error in errors: + print(f" schema violation in {document_path}: {list(error.absolute_path)}: {error.message}") + return not errors + + assert valid("consumer/audit.sarif"), "multi-file SARIF failed the official schema" + assert valid("consumer/single.sarif"), "single-file SARIF failed the official schema" + assert valid("consumer/mixed.sarif"), "mixed unreadable SARIF failed the official schema" + + # Mixed run: the unreadable file stays visible as an invocation notification. + mixed = json.loads(pathlib.Path("consumer/mixed.sarif").read_text()) + invocation = mixed["runs"][0]["invocations"][0] + assert invocation["executionSuccessful"] is False + assert invocation["toolExecutionNotifications"][0]["level"] == "error" + + # Negative control: the pre-D1 layout (notifications on the run itself) + # must be rejected, or this guard is not actually testing anything. + pre_d1 = json.loads(pathlib.Path("consumer/mixed.sarif").read_text()) + pre_d1["runs"][0]["toolExecutionNotifications"] = pre_d1["runs"][0]["invocations"][0]["toolExecutionNotifications"] + violations = list(validator.iter_errors(pre_d1)) + assert violations, "negative control unexpectedly validated - the guard is blind" + print("negative control rejected pre-D1 output as expected") + PY diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..d1a7300 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,58 @@ +# Changelog + +All notable changes to this project will be documented in this file. +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). + +## [Unreleased] + +### Fixed + +- The composite GitHub Action installs SheetSentry from the action's own + checked-out source (`github.action_path`, passed through a step environment + variable and a quoted shell argument) instead of `pip install sheetsentry`, + which targeted a PyPI package that does not exist and always failed. +- Aggregate SARIF (multi-file or failed audits) is valid against the official + OASIS SARIF 2.1.0 schema: tool execution notifications moved from the run + level into `run.invocations[0]` with an `executionSuccessful` flag (the run + object forbids additional properties). CI validates every generated SARIF + document against the pinned official schema, including a negative control + that must reject the old layout. +- Auditing several files with `--format json` or `--format sarif` now writes + one valid document instead of concatenated per-file documents. The JSON + multi-file shape is an envelope `{"reports": [...], "errors": [...]}` whose + entries keep the single-file payload; `errors` lists unreadable files. The + multi-file SARIF document has one run with deduplicated rule descriptors, + results carrying the original workbook path (directories included, spaces + URI-encoded) and an `artifacts` list that includes clean workbooks; + unreadable files become tool execution notifications. A single-file audit + keeps the original shapes. Exit priority is unchanged: clean 0, findings at + or above the threshold 1, unreadable input 2 even with `--fail-on never`. +- `review` given a multi-file JSON envelope now exits 2 with an explanation + instead of failing on reconciliation. +- The Action `files` input accepts one literal path containing spaces (used + as-is when it names exactly one existing file); glob/list behavior is + otherwise unchanged. + +### Changed + +- SARIF `informationUri` now points at the real source repository + (https://github.com/HlinorAI/sheetsentry). + +### Added + +- CI runs an `action-smoke` job that executes the real composite action from a + separate checkout in a synthetic consumer workspace: asserts SheetSentry is + not preinstalled, generates workbooks including a filename with spaces and + distinct per-file findings, exercises the default recursive pattern, a + literal spaced path and a JSON envelope, validates the SARIF report, the + envelope mapping and the installed-source metadata (`direct_url.json`). The + 3 OS x 4 Python test matrix is unchanged. +- `README.md` documents source-only installation (private repo, Python + 3.10+), the composite Action (same-repo and cross-repo private patterns + with a contents-read token placeholder), the `files` input semantics, and + the multi-file output shapes with the single-file `review` limitation. + +### History note + +No dependency, version, or CLI flag changes in this update; the 0.1.0 +package remains unpublished. diff --git a/PROJECT_PLAN.md b/PROJECT_PLAN.md new file mode 100644 index 0000000..7449d10 --- /dev/null +++ b/PROJECT_PLAN.md @@ -0,0 +1,49 @@ +# PROJECT_PLAN — SheetSentry + +Checks and code review for business-critical `.xlsx` files crossing a company +boundary: 17 audit rules, 8 diff rules, a finding-review workflow, and SARIF +output for GitHub code scanning. One runtime dependency (`openpyxl`); +everything else is the standard library. Python 3.10+. + +## Current state (2026-10-01) + +- Private repository, version `0.1.0`, **not published** to PyPI. +- Full test suite: 96 tests, CI matrix 3 OS x 4 Python versions. +- Composite GitHub Action (`action.yml`) runs the CLI on the caller's + workbooks and installs SheetSentry **from the action's own checked-out + source** (`github.action_path`) — not from PyPI, where no package exists. + +## Current work (SHEETSENTRY-001) + +- `action.yml`: installation step now installs the exact checked-out action + source via a step environment variable; no PyPI lookup. +- `ci.yml`: an `action-smoke` job runs the real composite action from a + separate checkout in a synthetic consumer workspace, starting from a + clean environment (asserts SheetSentry is not preinstalled), validates the + SARIF report and the installed-source metadata (`direct_url.json`). +- `README.md`: Install / In CI sections document source installation only and + the Python 3.10+ / private-repo prerequisites. +- No version bump, no dependency changes, no release in this change. + +## Multi-file output (SHEETSENTRY-002) + +- `audit` over several files emits one JSON envelope `{"reports": [...], + "errors": [...]}` (per-file payloads keep the single-file shape; unreadable + files are listed under `errors`) and one SARIF 2.1.0 document with a single + run: deduplicated rules, results carrying the original workbook path with + spaces URI-encoded, `artifacts` including clean workbooks, unreadable files + as tool execution notifications. Single-file audits keep the original + shapes. Exit priority: 0 clean / 1 findings / 2 unreadable input (even with + `--fail-on never`). +- `review` is single-file by design; a multi-file envelope exits 2 with an + explanation instead of mixing finding IDs across files. +- The Action `files` input takes a glob, a list, or one literal path + (spaces supported when it names exactly one existing file). + +## Governance + +This repository is governed by the portfolio agent contract and coordination +workflow, which live outside this repository (shared workspace, not published +here). Product decisions and Git operations follow that contract: bounded +implementation tasks, independent review, explicit Owner acceptance, Git-only +remote stages. diff --git a/README.md b/README.md index e75ec17..f52f456 100644 --- a/README.md +++ b/README.md @@ -35,8 +35,19 @@ Total: 3 findings (2 errors, 1 warning) ## Install +SheetSentry is not on PyPI. Install from a checkout of this repository +(requires access to this private repo and Python 3.10+): + +```bash +git clone https://github.com/HlinorAI/sheetsentry.git +cd sheetsentry +pip install . +``` + +or straight from the repository URL: + ```bash -pip install sheetsentry +pip install git+https://github.com/HlinorAI/sheetsentry.git ``` One dependency, `openpyxl`. Nothing else comes along. @@ -86,14 +97,76 @@ Exit codes: `0` clean, `1` findings at or above `--fail-on` (default `error`), ` ### In CI +The repository ships a composite GitHub Action that installs SheetSentry from +the action's own checked-out source — nothing is fetched from PyPI, because +there is no SheetSentry package there. The consumer workflow needs Python +3.10+ set up first (the action installs with `pip`), and access to this +private repository. + +Same repository: + +```yaml +- uses: actions/setup-python@v5 + with: + python-version: "3.11" +- uses: ./ # this repo's action.yml + with: + files: data/*.xlsx +- uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: sheetsentry.sarif +``` + +Another (private) repository: check out the consumer repo first, then the +action source, using a token that has **contents: read** on this repository +(create the token yourself; the examples below only reference it): + ```yaml -- run: pip install sheetsentry -- run: sheetsentry audit data/*.xlsx --format sarif -o results.sarif +- uses: actions/checkout@v4 # the consumer repository +- uses: actions/checkout@v4 # the action's source + with: + repository: HlinorAI/sheetsentry + token: ${{ secrets.SHEETSSENTRY_SOURCE_READ }} + path: sheetsentry-action +- uses: actions/setup-python@v5 + with: + python-version: "3.11" +- uses: ./sheetsentry-action + with: + files: data/*.xlsx - uses: github/codeql-action/upload-sarif@v3 with: - sarif_file: results.sarif + sarif_file: sheetsentry.sarif ``` +`files` accepts a glob (`data/*.xlsx`, recursive with `**`), several +space-separated patterns, or one literal path — a path containing spaces +works when it names exactly one existing file: + +```yaml +- uses: ./sheetsentry-action + with: + files: reports/final report.xlsx +``` + +#### Multi-file output + +Auditing several files at once produces **one** JSON document or **one** +SARIF document, not a concatenation: + +- JSON is an envelope `{"reports": [...], "errors": [...]}` where each entry + of `reports` has the same shape as a single-file report (`file`, `subject`, + `summary`, `findings`); `errors` lists files that could not be opened. + A single-file audit keeps the original root shape. +- SARIF 2.1.0 has one run; results carry the original workbook path + (directories included, spaces URI-encoded), rule descriptors are + deduplicated, and every successfully processed workbook — clean ones too — + is listed under `artifacts`. Unreadable files become tool execution + notifications instead of silently disappearing. +- The `review` command works on a single-file JSON report. Given a multi-file + envelope it exits with code 2 and an explanation instead of mixing findings + from different files; extract the entry you need from `reports` first. + ## What gets checked **Data and exchange (`SD`)** - what breaks the receiving system: diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..92963b8 --- /dev/null +++ b/TODO.md @@ -0,0 +1,12 @@ +# TODO — SheetSentry + +- [ ] Decide on PyPI publication (requires explicit Owner authorization; the + package name is currently free) and a matching release workflow. +- [ ] After publication: consider GitHub Marketplace listing for the composite + Action; align the package version with the shipped rule set (`diff` and + the review workflow are already in `0.1.0` but were added after the + initial release semantics) before any public release. +- [ ] Consider a `sheetsentry test` subcommand (currently a documented stub). +- [ ] Add lint/type checking and a coverage report to CI. +- [ ] Consider a `review` mode that understands multi-file envelopes + (per-file extraction) without cross-file finding-ID collisions. diff --git a/action.yml b/action.yml index 6078965..385f30a 100644 --- a/action.yml +++ b/action.yml @@ -8,7 +8,7 @@ branding: inputs: files: - description: Which files to check (glob) + description: Which files to check - a glob, a space-separated list, or one literal path (spaces supported) required: false default: "**/*.xlsx" fail-on: @@ -38,9 +38,11 @@ outputs: runs: using: composite steps: - - name: Install SheetSentry + - name: Install SheetSentry from the checked-out action source shell: bash - run: pip install --quiet sheetsentry + env: + SS_ACTION_PATH: ${{ github.action_path }} + run: python -m pip install --quiet "$SS_ACTION_PATH" - name: Check spreadsheets shell: bash @@ -57,7 +59,14 @@ runs: # Inputs travel through the environment rather than being interpolated # into the script: a glob expanded by the runner would be re-split by # the shell, and anything else would be executed. - files=($SS_FILES) + # If the whole input names exactly one existing file, it is taken as + # one literal path - spaces included. Anything else is a glob/list + # and expands as before. + if [ -f "$SS_FILES" ]; then + files=("$SS_FILES") + else + files=($SS_FILES) + fi if [ ${#files[@]} -eq 0 ]; then echo "SheetSentry: no matching files, skipping" exit 0 diff --git a/src/sheetsentry/cli.py b/src/sheetsentry/cli.py index 2352a03..7ee4063 100644 --- a/src/sheetsentry/cli.py +++ b/src/sheetsentry/cli.py @@ -71,9 +71,11 @@ def cmd_diff(args: argparse.Namespace) -> int: def cmd_audit(args: argparse.Namespace) -> int: threshold = Severity(args.fail_on) if args.fail_on != "never" else None - outputs: list[str] = [] worst_hit = False failed = False + # (path, findings, error) per requested file; error is the reason the + # workbook could not be opened, findings is None then. + entries: list[tuple[Path, list | None, str | None]] = [] for name in args.files: path = Path(name) @@ -81,15 +83,34 @@ def cmd_audit(args: argparse.Namespace) -> int: if error: print(error, file=sys.stderr) failed = True + entries.append((path, None, error)) continue findings = checks.run(book, select=_split(args.select), ignore=_split(args.ignore)) - outputs.append(report.render(args.format, path, findings)) + entries.append((path, findings, None)) if threshold and any(at_least(f.severity, threshold) for f in findings): worst_hit = True - body = "\n\n".join(outputs) + if args.format == "json": + if len(entries) == 1 and entries[0][2] is None: + body = report.as_json(entries[0][0], entries[0][1]) + else: + body = report.aggregate_json(entries) + elif args.format == "sarif": + if len(entries) == 1 and entries[0][2] is None: + body = report.as_sarif(entries[0][0], entries[0][1]) + else: + body = report.aggregate_sarif(entries) + else: + # Text stays one human-readable block per processed file; unreadable + # files are already explained on stderr. + body = "\n\n".join( + report.render("text", path, findings) + for path, findings, error in entries + if error is None + ) + if args.output: Path(args.output).write_text(body + "\n", encoding="utf-8") else: @@ -114,6 +135,14 @@ def cmd_rules(args: argparse.Namespace) -> int: def cmd_review(args: argparse.Namespace) -> int: try: report_payload = json.loads(Path(args.report).read_text(encoding="utf-8")) + if isinstance(report_payload, dict) and "reports" in report_payload: + print( + "sheetsentry: review expects a single-file audit report; this file is a " + "multi-file envelope. Its per-file payloads live in the 'reports' array - " + "run 'audit' on one file with --format json to get a reviewable report.", + file=sys.stderr, + ) + return EXIT_ERROR state = load_state(Path(args.state)) result = reconcile(report_payload, state) except (OSError, json.JSONDecodeError, ValueError) as exc: diff --git a/src/sheetsentry/report.py b/src/sheetsentry/report.py index b2c0ddc..e99fd04 100644 --- a/src/sheetsentry/report.py +++ b/src/sheetsentry/report.py @@ -8,6 +8,7 @@ from __future__ import annotations import json +import urllib.parse from collections import Counter from pathlib import Path @@ -15,6 +16,8 @@ from .finding import Finding, Severity from .text import n_plural +_INFORMATION_URI = "https://github.com/HlinorAI/sheetsentry" + _HEADING = { Severity.ERROR: "ERRORS", Severity.WARNING: "WARNINGS", @@ -124,7 +127,7 @@ def as_sarif(path: Path, findings: list[Finding], title: str | None = None) -> s "tool": { "driver": { "name": "SheetSentry", - "informationUri": "https://github.com/sheetsentry/sheetsentry", + "informationUri": _INFORMATION_URI, "rules": rules, } }, @@ -137,6 +140,137 @@ def as_sarif(path: Path, findings: list[Finding], title: str | None = None) -> s ) +def _artifact_uri(path: Path) -> str: + """Relative forward-slash URI for a workbook, spaces encoded. + + The full given path (directories included) is kept so that two files with + the same basename in different directories stay distinguishable. + """ + return urllib.parse.quote(path.as_posix(), safe="/:") + + +def _sarif_rules(findings_lists: list[list[Finding]]) -> list[dict]: + """One descriptor per distinct ruleId across all inputs, deduplicated.""" + known = {**REGISTRY, **DIFF_REGISTRY} + used = sorted({finding.rule for findings in findings_lists for finding in findings}) + return [ + { + "id": rule, + "name": known[rule].title, + "shortDescription": {"text": known[rule].title}, + "fullDescription": {"text": known[rule].summary}, + "defaultConfiguration": {"level": _SARIF_LEVEL[known[rule].severity]}, + } + for rule in used + if rule in known + ] + + +def aggregate_json( + entries: list[tuple[Path, list[Finding] | None, str | None]], +) -> str: + """One JSON object for several workbooks (or any failed single run). + + `reports` holds the usual per-file payload for every successfully read + workbook; `errors` records the files that could not be opened, so a bad + file is never presented as a clean one. Single-file successful audits keep + the original root shape instead. + """ + reports: list[dict] = [] + errors: list[dict] = [] + for path, findings, error in entries: + if error is not None: + errors.append({"file": str(path), "error": error}) + else: + reports.append(json.loads(as_json(path, findings or []))) + return json.dumps( + {"reports": reports, "errors": errors}, ensure_ascii=False, indent=2 + ) + + +def aggregate_sarif( + entries: list[tuple[Path, list[Finding] | None, str | None]], +) -> str: + """One SARIF 2.1.0 document for several workbooks (or any failed run). + + One run; rule descriptors deduplicated across inputs; results keep the + original workbook path and the cell address in the message. `artifacts` + lists every successfully processed workbook, clean ones included, so the + coverage of multiple inputs is observable even without findings. Files + that could not be opened become tool execution notifications instead of + silently disappearing. + """ + successful = [ + (path, findings or []) for path, findings, error in entries if error is None + ] + rules = _sarif_rules([findings for _, findings in successful]) + + results: list[dict] = [] + artifacts: list[dict] = [] + notifications: list[dict] = [] + seen_uris: set[str] = set() + for path, findings, error in entries: + if error is not None: + notifications.append( + { + "level": "error", + "message": {"text": f"{path}: {error}"}, + } + ) + continue + uri = _artifact_uri(path) + if uri not in seen_uris: + seen_uris.add(uri) + artifacts.append({"location": {"uri": uri}}) + for finding in findings: + results.append( + { + "ruleId": finding.rule, + "level": _SARIF_LEVEL[finding.severity], + "message": {"text": f"{finding.location()}: {finding.message}"}, + "locations": [ + { + "physicalLocation": { + "artifactLocation": {"uri": uri}, + "region": {"startLine": 1}, + } + } + ], + } + ) + + return json.dumps( + { + "$schema": "https://json.schemastore.org/sarif-2.1.0.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "SheetSentry", + "informationUri": _INFORMATION_URI, + "rules": rules, + } + }, + "invocations": [ + { + # SARIF 2.1.0 keeps tool execution notifications + # under run.invocations[], not on the run itself + # (the run object forbids additional properties). + "executionSuccessful": not notifications, + "toolExecutionNotifications": notifications, + } + ], + "artifacts": artifacts, + "results": results, + } + ], + }, + ensure_ascii=False, + indent=2, + ) + + def render(fmt: str, path: Path, findings: list[Finding], title: str | None = None) -> str: if fmt == "json": return as_json(path, findings, title) diff --git a/tests/test_cli.py b/tests/test_cli.py index 99a690e..5a59af3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -7,8 +7,9 @@ from sheetsentry.cli import EXIT_ERROR, EXIT_FINDINGS, EXIT_OK, main -def broken(build): +def broken(build, name="case.xlsx"): return build( + name=name, Budget=[ ["Item", "Amount"], ["rent", 100], @@ -20,8 +21,8 @@ def broken(build): ) -def clean(build): - return build(Sheet=[["Item", "Amount"], ["rent", 100], ["telecom", 200]]) +def clean(build, name="case.xlsx"): + return build(name=name, Sheet=[["Item", "Amount"], ["rent", 100], ["telecom", 200]]) class TestExitCodes: @@ -114,3 +115,150 @@ def test_review_reconciles_resolved_finding(tmp_path, build, capsys): def test_unimplemented_commands_say_when(capsys): assert main(["test", "a.xlsx"]) == EXIT_ERROR assert "0.3" in capsys.readouterr().err + + +class TestMultiFileAudit: + """Multi-file audit: one valid structured document, files kept apart.""" + + def _dupes(self, build, name="dupes.xlsx"): + # Same shape tests/test_checks_data.py uses for a real SD003 error: + # a recognized key column with a duplicated value. + rows = [[f"A{i}", "item", i] for i in range(1, 25)] + rows[9][0] = "A1" + return build(name=name, Price=[["SKU", "Name", "Price"]] + rows) + + def test_multi_file_json_is_one_envelope_mapping_each_file(self, build, capsys): + good = clean(build, name="good.xlsx") + dupes = self._dupes(build) + assert main(["audit", str(good), str(dupes), "--format", "json"]) == EXIT_FINDINGS + payload = json.loads(capsys.readouterr().out) # exactly one document + + assert [r["file"] for r in payload["reports"]] == [str(good), str(dupes)] + by_file = {r["file"]: r for r in payload["reports"]} + assert by_file[str(good)]["summary"]["total"] == 0 + assert by_file[str(dupes)]["summary"]["error"] == 1 + assert payload["errors"] == [] + # Single-file root shape is preserved inside the envelope. + assert by_file[str(dupes)]["findings"][0]["rule"] == "SD003" + + def test_multi_file_json_output_file_parses_once(self, tmp_path, build): + a, b = clean(build, name="a.xlsx"), self._dupes(build) + out = tmp_path / "out.json" + assert main(["audit", str(a), str(b), "--format", "json", "-o", str(out)]) == EXIT_FINDINGS + payload = json.loads(out.read_text(encoding="utf-8")) + assert len(payload["reports"]) == 2 + + def test_multi_file_sarif_is_one_document_with_file_uris(self, tmp_path, build, capsys): + import urllib.parse + + dupes = self._dupes(build) + subdir = tmp_path / "sub dir" # space in directory name + subdir.mkdir() + (subdir / "year end.xlsx").write_bytes(dupes.read_bytes()) + good = clean(build, name="good.xlsx") + + def uri(path): + return urllib.parse.quote(path.as_posix(), safe="/:") + + assert main( + ["audit", str(good), str(dupes), str(subdir / "year end.xlsx"), + "--format", "sarif"] + ) == EXIT_FINDINGS + payload = json.loads(capsys.readouterr().out) # exactly one document + assert payload["version"] == "2.1.0" + run = payload["runs"][0] + assert len(payload["runs"]) == 1 + result_uris = { + loc["physicalLocation"]["artifactLocation"]["uri"] + for res in run["results"] for loc in res["locations"] + } + assert uri(subdir / "year end.xlsx") in result_uris # space encoded, dir kept + assert uri(dupes) in result_uris + rule_ids = {rule["id"] for rule in run["tool"]["driver"]["rules"]} + assert rule_ids == {"SD003"} # deduplicated + artifact_uris = {a["location"]["uri"] for a in run["artifacts"]} + assert artifact_uris == {uri(good), uri(dupes), uri(subdir / "year end.xlsx")} + assert run["tool"]["driver"]["informationUri"] == "https://github.com/HlinorAI/sheetsentry" + + def test_same_basename_in_distinct_dirs_stays_distinguishable(self, tmp_path, build, capsys): + import urllib.parse + + def uri(path): + return urllib.parse.quote(path.as_posix(), safe="/:") + + one = tmp_path / "a"; one.mkdir() + two = tmp_path / "b"; two.mkdir() + (one / "same.xlsx").write_bytes(self._dupes(build, "one.xlsx").read_bytes()) + (two / "same.xlsx").write_bytes(clean(build, "two.xlsx").read_bytes()) + assert main(["audit", str(one / "same.xlsx"), str(two / "same.xlsx"), + "--format", "sarif"]) == EXIT_FINDINGS + run = json.loads(capsys.readouterr().out)["runs"][0] + result_uris = {loc["physicalLocation"]["artifactLocation"]["uri"] + for res in run["results"] for loc in res["locations"]} + assert result_uris == {uri(one / "same.xlsx")} # only this one has findings + artifact_uris = {a["location"]["uri"] for a in run["artifacts"]} + assert artifact_uris == {uri(one / "same.xlsx"), uri(two / "same.xlsx")} + + def test_mixed_good_and_bad_is_exit_two_with_honest_envelope(self, tmp_path, build, capsys): + good = clean(build, name="good.xlsx") + bad = tmp_path / "broken.xlsx" + bad.write_bytes(b"not a workbook") + assert main(["audit", str(good), str(bad), "--format", "json"]) == EXIT_ERROR + payload = json.loads(capsys.readouterr().out) + assert [r["file"] for r in payload["reports"]] == [str(good)] + assert len(payload["errors"]) == 1 + assert payload["errors"][0]["file"] == str(bad) + assert payload["errors"][0]["error"] + + def test_all_bad_is_exit_two_with_valid_empty_envelope(self, tmp_path, capsys): + bad1, bad2 = tmp_path / "x.xlsx", tmp_path / "y.xlsx" + bad1.write_bytes(b"nope"); bad2.write_bytes(b"nope") + assert main(["audit", str(bad1), str(bad2), "--format", "json"]) == EXIT_ERROR + payload = json.loads(capsys.readouterr().out) + assert payload["reports"] == [] + assert [e["file"] for e in payload["errors"]] == [str(bad1), str(bad2)] + + def test_sarif_failure_becomes_invocation_notification_not_silence(self, tmp_path, build, capsys): + bad = tmp_path / "broken.xlsx" + bad.write_bytes(b"nope") + good = clean(build, name="good.xlsx") + assert main(["audit", str(good), str(bad), "--format", "sarif"]) == EXIT_ERROR + run = json.loads(capsys.readouterr().out)["runs"][0] + assert run["results"] == [] # good file is clean, not hidden + assert len(run["artifacts"]) == 1 + # SARIF 2.1.0: notifications live under run.invocations[], never on the run. + invocation = run["invocations"][0] + assert invocation["executionSuccessful"] is False + assert invocation["toolExecutionNotifications"][0]["level"] == "error" + assert "toolExecutionNotifications" not in run # no unsupported run-level key + + def test_sarif_clean_multifile_invocation_reports_success(self, tmp_path, build, capsys): + a = clean(build, name="a.xlsx") + b = clean(build, name="b.xlsx") + assert main(["audit", str(a), str(b), "--format", "sarif"]) == EXIT_OK + run = json.loads(capsys.readouterr().out)["runs"][0] + invocation = run["invocations"][0] + assert invocation["executionSuccessful"] is True + assert invocation["toolExecutionNotifications"] == [] + assert "toolExecutionNotifications" not in run # no unsupported run-level key + assert len(run["artifacts"]) == 2 # both clean files observable + + def test_fail_on_never_still_exits_two_on_unreadable_file(self, tmp_path, build): + bad = tmp_path / "broken.xlsx" + bad.write_bytes(b"nope") + assert main(["audit", str(bad), "--fail-on", "never", "--format", "json"]) == EXIT_ERROR + + def test_single_file_json_keeps_root_shape(self, build, capsys): + good = clean(build, name="good.xlsx") + assert main(["audit", str(good), "--format", "json"]) == EXIT_OK + payload = json.loads(capsys.readouterr().out) + assert set(payload) == {"file", "subject", "summary", "findings"} + + def test_review_rejects_multi_file_envelope_with_clear_error(self, tmp_path, build, capsys): + a, b = clean(build, name="a.xlsx"), self._dupes(build) + envelope = tmp_path / "envelope.json" + assert main(["audit", str(a), str(b), "--format", "json", "-o", str(envelope)]) == EXIT_FINDINGS + state = tmp_path / "state.json" + state.write_text(json.dumps({"version": 1, "items": []}), encoding="utf-8") + assert main(["review", str(envelope), "--state", str(state)]) == EXIT_ERROR + assert "multi-file envelope" in capsys.readouterr().err From d6da40ca95e4d81804b94ff648a7d2d2bf7eebd1 Mon Sep 17 00:00:00 2001 From: Ishavn Date: Fri, 2 Oct 2026 10:11:16 +0300 Subject: [PATCH 2/2] Check expected finding instead of brittle warning count --- .github/workflows/ci.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c69342a..cf6b2a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -174,7 +174,16 @@ jobs: # Literal path with spaces: single-file root shape. literal = json.loads(pathlib.Path("consumer/literal.json").read_text()) assert literal["file"] == "consumer/sub dir/year end.xlsx", literal["file"] - assert literal["summary"]["warning"] == 1 + # The SD001 text-number finding must be present; other legitimate + # rules (SD004 mixed types fires on this same fixture) are allowed + # and only have to agree with the summary counts. + rules = {finding["rule"]: finding["severity"] for finding in literal["findings"]} + assert rules.get("SD001") == "warning", rules + assert literal["summary"]["warning"] == sum( + 1 for finding in literal["findings"] if finding["severity"] == "warning" + ) + assert literal["summary"]["total"] == len(literal["findings"]) + assert literal["summary"]["error"] == 0 print("Literal spaced path OK:", literal["file"]) PY