-
Notifications
You must be signed in to change notification settings - Fork 1
feat(audit): pack-conformance auditor + SARIF 2.1.0 emitter #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2964538
349cccd
0a12aa3
e752727
360f103
e6e9d11
e0e83d4
ebdb07f
787fe1d
83d9376
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,144 @@ | ||||||||||
| # `nboot audit` — pack-conformance audit | ||||||||||
|
|
||||||||||
| Check whether an existing project still matches a navi-bootstrap pack, and | ||||||||||
| emit findings as either human-readable text or SARIF 2.1.0 for upload to | ||||||||||
| GitHub's Security tab. | ||||||||||
|
|
||||||||||
| ## Why | ||||||||||
|
|
||||||||||
| Most templating tools (Cookiecutter, Backstage Scaffolder, Yeoman) are | ||||||||||
| **create-only** — they generate a project once, then walk away. Copier has | ||||||||||
| an `update` flow but depends on the target having been Copier-generated in | ||||||||||
| the first place. | ||||||||||
|
|
||||||||||
| `nboot audit` flips the problem: it re-renders a pack **in memory** and | ||||||||||
| compares the output to an existing project's files on disk. No merge, no | ||||||||||
| write, no state. You get a list of files that are missing or drifted from | ||||||||||
| the pack — the template becomes a living specification. | ||||||||||
|
|
||||||||||
| This is especially useful for: | ||||||||||
|
|
||||||||||
| - **Fleet surveys** — "which of our 100 repos still conform to the | ||||||||||
| `security-scanning` pack?" | ||||||||||
| - **CI gates** — run `nboot audit … --format sarif` in a nightly job and | ||||||||||
| upload via [`github/codeql-action/upload-sarif`] so drift appears in the | ||||||||||
| Security tab alongside CodeQL and Semgrep. | ||||||||||
| - **Regression detection** — after a bulk refactor, confirm no workflow or | ||||||||||
| pre-commit config silently fell out of conformance. | ||||||||||
|
|
||||||||||
| [`github/codeql-action/upload-sarif`]: https://github.com/github/codeql-action | ||||||||||
|
|
||||||||||
| ## Usage | ||||||||||
|
|
||||||||||
| ```bash | ||||||||||
| uv run nboot audit \ | ||||||||||
| --spec nboot-spec.json \ | ||||||||||
| --pack security-scanning \ | ||||||||||
| --target /path/to/existing/project | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| Drift is reported and the command exits non-zero so CI fails: | ||||||||||
|
|
||||||||||
| ``` | ||||||||||
| Audit found 3 drift finding(s): | ||||||||||
|
|
||||||||||
| Missing files (2): | ||||||||||
| - .github/workflows/codeql.yml | ||||||||||
| - .github/workflows/scorecard.yml | ||||||||||
|
|
||||||||||
| Changed files (1): | ||||||||||
| - .github/dependabot.yml | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ## Flags | ||||||||||
|
|
||||||||||
| | Flag | Default | Effect | | ||||||||||
| |---|---|---| | ||||||||||
| | `--spec` | (required) | Path to the project spec JSON | | ||||||||||
| | `--pack` | (required) | Pack name to audit against (`scaffold`, `base`, `security-scanning`, …) | | ||||||||||
| | `--target` | (required) | Existing project directory to inspect | | ||||||||||
| | `--format` | `text` | `text` for humans, `sarif` for GitHub Security tab | | ||||||||||
| | `--output` | stdout | Write to a file instead of stdout (useful with `--format sarif`) | | ||||||||||
| | `--resolve` | off | Resolve action SHAs via `gh` before planning (default: offline) | | ||||||||||
| | `--exit-zero` | off | Exit 0 even when drift is found (report-only CI surveys) | | ||||||||||
|
|
||||||||||
| By default `audit` runs **offline** — no GitHub API calls. This lets fleet | ||||||||||
| audits run reliably from air-gapped or rate-limited environments. Pass | ||||||||||
| `--resolve` if the pack's rendered output depends on freshly-resolved | ||||||||||
| action SHAs. | ||||||||||
|
|
||||||||||
| ## SARIF output | ||||||||||
|
|
||||||||||
| The SARIF 2.1.0 report declares two rules: | ||||||||||
|
|
||||||||||
| - `pack-drift-missing` — file expected by the pack but absent from the target | ||||||||||
| - `pack-drift-changed` — file content differs from the pack's rendered output | ||||||||||
|
|
||||||||||
| Each finding includes a stable `partialFingerprints.primaryLocationLineHash` | ||||||||||
| so GitHub's Security tab deduplicates across runs. | ||||||||||
|
|
||||||||||
| Upload it in CI: | ||||||||||
|
|
||||||||||
| ```yaml | ||||||||||
| - name: Audit against security-scanning pack | ||||||||||
| run: | | ||||||||||
| uv run nboot audit \ | ||||||||||
| --spec nboot-spec.json \ | ||||||||||
| --pack security-scanning \ | ||||||||||
| --target . \ | ||||||||||
| --format sarif \ | ||||||||||
| --output audit.sarif.json \ | ||||||||||
| --exit-zero | ||||||||||
|
|
||||||||||
| - name: Upload audit findings | ||||||||||
| uses: github/codeql-action/upload-sarif@v4 | ||||||||||
|
Fieldnote-Echo marked this conversation as resolved.
|
||||||||||
| with: | ||||||||||
| sarif_file: audit.sarif.json | ||||||||||
| category: nboot-audit | ||||||||||
| ``` | ||||||||||
|
|
||||||||||
| ## Threat model and operational notes | ||||||||||
|
|
||||||||||
| `nboot audit` is a **defence-in-depth** tool, not a hardened sandbox. The | ||||||||||
| path-confinement check in `compute_diffs` resolves every destination | ||||||||||
| relative to the target directory and rejects traversal, absolute paths, | ||||||||||
| and symlink escapes — but it operates at the moment the audit runs. | ||||||||||
|
|
||||||||||
| Known limits: | ||||||||||
|
|
||||||||||
| - **TOCTOU.** The check resolves paths once; in a shared or mutable | ||||||||||
| environment a path may flip from safe to unsafe between the check | ||||||||||
| and any subsequent read. For audits that matter (CI gates, fleet | ||||||||||
| surveys), run against a freshly-cloned working tree or a read-only | ||||||||||
| mount. | ||||||||||
| - **Chained symlinks created mid-run.** If another process creates new | ||||||||||
| symlinks under `--target` while audit is iterating, files added after | ||||||||||
| the resolve check are not re-confined. Same mitigation: avoid running | ||||||||||
| audit on a directory another process is actively writing to. | ||||||||||
| - **Privilege.** Run audit with the lowest privilege that can read the | ||||||||||
| target. Don't run as root unless the target requires it. | ||||||||||
|
|
||||||||||
| For most CI usage — clone, audit, exit — these limits don't apply. They | ||||||||||
| only matter if the audit is exposed to an attacker who can mutate the | ||||||||||
| target while audit is running. | ||||||||||
|
|
||||||||||
| ## Exit codes | ||||||||||
|
|
||||||||||
| | Exit | Meaning | | ||||||||||
| |---|---| | ||||||||||
| | 0 | Target fully conforms to the pack, OR drift found with `--exit-zero` | | ||||||||||
| | 1 | Drift found without `--exit-zero` | | ||||||||||
| | 2 | Pipeline error (bad spec, missing pack, path-confinement violation, template render failure). Always emitted to stderr, never suppressed by `--exit-zero` | | ||||||||||
|
|
||||||||||
| Exit 1 vs 2 lets CI distinguish "the audit ran and reported drift" from "the | ||||||||||
| audit failed to run". Wire your pipeline so only exit 1 gates the merge. | ||||||||||
|
||||||||||
| audit failed to run". Wire your pipeline so only exit 1 gates the merge. | |
| audit failed to run". In CI, fail the job on any non-zero exit code; use the | |
| 1 vs 2 distinction only for reporting, alerting, or classifying drift versus | |
| pipeline failure. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,7 @@ dependencies = [ | |
| "click>=8.1.0", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 LOW: Dependency pinning does not specify upper boundsConfidence: 88% All dependencies use only minimum bound (>=) with no upper limit. This can introduce forward compatibility breakage if a future dependency release introduces incompatible changes. Suggestion: Consider adding upper version bounds to all runtime dependencies in pyproject.toml to prevent accidental breaking changes from newly released major versions. — No upper bounds means trusting every dependency forever. Sometimes that's fine; sometimes that's a fire drill. |
||
| "jinja2>=3.1.0", | ||
| "jsonschema>=4.20.0", | ||
| "navi-sanitize>=0.1.0", | ||
| "navi-sanitize>=0.2.1", | ||
| "pyyaml>=6.0", | ||
| ] | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,9 +3,24 @@ | |
|
|
||
|
Fieldnote-Echo marked this conversation as resolved.
|
||
| """navi-bootstrap: Jinja2 rendering engine and template packs.""" | ||
|
|
||
| from importlib.metadata import PackageNotFoundError | ||
|
Fieldnote-Echo marked this conversation as resolved.
|
||
| from importlib.metadata import version as _pkg_version | ||
|
|
||
| from navi_bootstrap.packs import get_ordered_packs | ||
|
Fieldnote-Echo marked this conversation as resolved.
|
||
| from navi_bootstrap.spec import build_spec_for_new | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 LOW: Governance: version exported in all and test coverage caveatConfidence: 95% Exporting version via all is sound, but the comment on the 'pragma: no cover' for the importlib.metadata fallback warrants a code comment indicating that this is intentional and not an accidental test skip. Suggestion: Add a brief comment explaining that test coverage of the fallback is intentionally skipped because it only triggers in non-packaged dev environments. — The pragma is fine, but a comment for future maintainers would prevent awkward codecov questions. |
||
|
|
||
|
Fieldnote-Echo marked this conversation as resolved.
|
||
| __all__ = ["build_spec_for_new", "get_ordered_packs"] | ||
| # Single-source the version from installed package metadata so __init__.py | ||
| # tracks pyproject.toml automatically. The fallback only triggers when the | ||
| # package isn't installed (uncommon: editable-install dev sessions where the | ||
| # package was deleted, or a source checkout being imported via PYTHONPATH | ||
| # without `pip install -e .`). Downstream consumers — notably the SARIF | ||
| # `tool.driver.version` field emitted by `nboot audit` — must tolerate the | ||
| # `0.0.0+unknown` form. The pragma excludes it from coverage because it | ||
| # only fires in that uncommon dev configuration and isn't worth simulating | ||
| # in tests. | ||
| try: | ||
| __version__ = _pkg_version("navi-bootstrap") | ||
| except PackageNotFoundError: # pragma: no cover — only during dev without install | ||
| __version__ = "0.0.0+unknown" | ||
|
|
||
| __version__ = "0.1.1" | ||
| __all__ = ["__version__", "build_spec_for_new", "get_ordered_packs"] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 LOW: Documentation omits SARIF rules extensibility caveat
Confidence: 90%
The documentation does not mention that only a fixed set of SARIF rules (missing/changed) is supported and that custom/errors for other drift types are not currently extensible. This could mislead consumers expecting more granular rule coverage.
Suggestion: Clarify in documentation that SARIF emission is intentionally limited to missing and changed files, and other kinds of drift will require schema extension.
— Not wrong, but future-you (or your users) might expect more than two SARIF rules.