Skip to content

hasp diff <base>: PR-delta mode - #7

Merged
electricapp merged 8 commits into
mainfrom
stack/04-diff
May 28, 2026
Merged

electricapp merged 8 commits into
mainfrom
stack/04-diff

Conversation

@electricapp

Copy link
Copy Markdown
Owner

New hasp diff <base> subcommand that runs the audit against a git worktree checked out at the base ref and against HEAD, then emits the finding-level delta.

Implementation (src/diff.rs):

  • BaseWorktree: Drop-guarded git worktree add --detach --no-checkout + git checkout <sha> so failed runs don't leak worktree directories.
  • FindingKey tuple (file, severity, title, detail, is_warning) drives the set-difference against both scans.
  • DeltaReport.exit_code() returns 1 iff the HEAD branch introduces a new deny-level finding — warn-only or fixed-only PRs exit clean.
  • Markdown renderer emits a summary table + <details> block suitable for gh pr comment --body-file -.
  • Minimal stdlib-only JSON emitter (no serde_json dep added).

Note: In this PR, the diff scan runs inline (not in a sandboxed subprocess). A later stacked PR moves it to --internal-scan subprocesses.

CLI (src/cli.rs):

  • New diff subcommand, Mode::Diff variant, parse_diff()
  • --format terse|markdown|json (default terse)
  • --dir / --policy / --no-policy / --paranoid / --allow-unsandboxed carry through

Refactor: Severity gains #[derive(Hash)] so FindingKey can live in a HashSet.

Tests: 5 unit + 4 integration (new finding, fixed finding, markdown format, json format). Full suite passes; clippy clean.

New check under `provenance.slsa-attestation` that verifies every pinned
`uses:` SHA has a matching SLSA build attestation on GitHub, goes beyond
zizmor's `known-vulnerable-actions` (CVE matching) to positive provenance
evidence.

Parser + verifier (src/github/slsa.rs):
  * Parses GitHub's /repos/{owner}/{repo}/attestations/{sha} envelope
  * Base64-decodes DSSE payload -> in-toto Statement
  * Validates predicateType is a SLSA provenance version (v0.2 / v1)
  * Confirms subject.digest.sha1/gitCommit/sha256 binds to the pinned SHA
  * Checks predicate.runDetails.builder.id is a trusted GitHub Actions
    builder identity
  * Returns a typed AttestationVerdict: Verified / Missing / SubjectMismatch
    / UntrustedBuilder / UnknownPredicate / MalformedAttestation

Cryptographic Sigstore signature verification is deferred to v2; v1 is
presence + predicate binding.

Proxy (src/proxy.rs):
  * New GET_ATTESTATION command
  * MAX_LARGE_MESSAGE_BYTES = 256 KiB response cap (attestation bundles
    are multi-KB each); opt-in per command via request_with_cap()

Client (src/github/client.rs):
  * Api::get_attestation(owner, repo, sha) -> Result<Option<String>> with a
    default Ok(None) impl so existing test mocks compile unchanged
  * Real impl calls attestations endpoint, 256 KiB body cap, treats 404
    as `None`

Provenance integration (src/github/provenance.rs):
  * emit_slsa_finding() translates AttestationVerdict into findings:
      - SubjectMismatch   -> CRIT (tampered / moved attestation)
      - UntrustedBuilder  -> HIGH
      - UnknownPredicate  -> MED
      - Malformed         -> MED
      - Missing           -> MED (default Warn; many actions haven't
                                  adopted SLSA yet)
  * Runs after existing tag-age-gap check, gated by the
    `provenance.slsa-attestation` policy level

Policy threading: `slsa-attestation` added to ProvenanceCheckConfig,
PartialProvenanceConfig, parse_provenance_config, parse_partial_provenance,
set_all_checks_deny, has_any_enabled_check, has_any_provenance_check, drift
detection, and check_name_for_finding. Default Warn; --paranoid forces Deny.

Tests: 6 unit tests covering all 6 verdict variants; 378 total tests pass;
clippy clean.
New `hasp diff <base>` subcommand that re-runs the static audit against
a temporary git worktree checked out at the base ref, then reports the
finding-level delta vs HEAD: new findings, fixed findings, unchanged
count.  Output formats: terse (default), markdown (GitHub-PR-comment
friendly), json (CI-consumable).

Implementation (src/diff.rs):
  * BaseWorktree: Drop-guarded `git worktree add --detach --no-checkout`
    + `git checkout <sha>` so failed runs don't leak worktree dirs.
  * FindingKey tuple (file, severity, title, detail, is_warning) drives
    set-difference against both scans.
  * DeltaReport.exit_code() returns 1 iff the HEAD branch introduces a
    new deny-level finding -- warn-only or fixed-only PRs exit clean.
  * Markdown renderer emits a summary table + <details> block suitable
    for `gh pr comment --body-file -`.
  * Minimal stdlib-only JSON emitter (no serde_json dep added).
  * Inline scan (not multi-process sandboxed) because delta mode is a
    developer convenience; users wanting the hardened sandbox run
    `hasp --paranoid` authoritatively on each branch.

CLI (src/cli.rs):
  * New `diff` subcommand, `Mode::Diff` variant, parse_diff() sibling
    of the existing parse_exec()
  * --format terse|markdown|json (default terse)
  * --dir / --policy / --no-policy / --paranoid / --allow-unsandboxed
    carry through for consistent behavior with the launcher
  * print_diff_help() for `hasp diff --help`

Refactor: promoted Severity to #[derive(Hash)] so FindingKey can live
in a HashSet. Refactored print_json into fmt::Write to silence
format_push_string lint.

Tests: 5 unit + 4 integration (new finding, fixed finding, markdown
format, json format), all green; 392 total tests pass; clippy clean
on --all-targets -- -D warnings.
- normalize finding.file to relative-to-scan-root so base/head paths match
- require --allow-unsandboxed; run integrity check on both scans
- pass -c core.hooksPath=/dev/null on every git invocation
- markdown listing uses fenced block; md escape covers < > ` \
- SIGINT/SIGTERM tears down temp worktree before exit
- unique unguessable worktree path, validated via create_dir
- short-circuit when base == HEAD
- drop dead fail_on_unchanged param
- extract git_util, Policy::resolve to share with main
- terse output uses {:<4} padding
- regression tests in tests/pr_delta.rs
@electricapp
electricapp changed the base branch from stack/03-slsa-v1 to main May 18, 2026 20:42
Refresh Cargo.lock to clear pending RUSTSEC advisories. Expand the
ureq 3 TODO to record that the bump is blocked on rewriting
github::client SPKI pinning behind a custom `Connector` (ureq 3
removed support for plugging in a `ServerCertVerifier`).
First-party `actions/*` (checkout, upload-artifact, etc.) and other
well-known publishers in the built-in `trusted_owners` list don't ship
SLSA attestations — `actions/attest-build-provenance` itself only emerged
recently and GitHub doesn't backfill it for its own first-party actions.
Flagging them as MED/deny was a dogfooding false positive that broke
self-scan on hasp's own workflows.
- diff: also strip canonical-root prefix from finding title/detail,
  not just file. cross_workflow/oidc audits embed `path.display()` in
  user-facing text; without this, base and head scans key differently
  and every persistent finding falsely shows up as both new and fixed.
- diff: signal-handler/Drop race fix. Lock-free OnceLock + AtomicBool
  so SIGINT during register/clear can't deadlock.
- diff: clear signal target before Drop runs.
- slsa: SubjectMismatch (tampered binding) always denies; the warn-by-
  default knob applies only to "no attestation present yet".
- slsa: parser failures honor the configured policy level instead of
  hardcoded warn.
- slsa: prefer SubjectMismatch > UntrustedBuilder > UnknownPredicate >
  MalformedAttestation when multiple attestations are present.
- slsa: reject cross-digest-type binding (sha1-keyed digest must match
  a sha1 expectation; sha256-keyed against sha256).
- provenance: dedupe SLSA fetch + parse per (owner,repo,sha) via the
  existing `checked` HashMap. Same SHA referenced N times → 1 call.
- proxy/client: raise large-message cap 256 KiB → 1 MiB for big SLSA
  bundles.
- diff: run audit::oidc::run in scan_and_audit so OIDC findings show up
  in the delta. `load_policy_acceptances` extracted to oidc/mod.rs so
  main and diff share one implementation. diff mode reads policy from
  .hasp.yml only (no CLI --oidc-policy flags exposed to `hasp diff`).
- diff: BaseWorktree::create now constructs Self before running git
  checkout, so a checkout failure still triggers Drop and tears down
  the registered worktree entry.
- diff: revert drop-order regression. drop(worktree) runs first;
  cleanup_worktree is idempotent so a signal landing between the two
  only triggers a harmless no-op second pass.
- diff: signal cleanup uses Mutex<Option> + AtomicBool fast path.
  Re-registration (future multi-base diff loops) now works; handler
  stays lock-free in the common case and uses try_lock under contention.
- slsa: reject cross-digest-type binding for unexpected hash lengths
  (Other → no match, fully explicit).
- provenance: SLSA-fetch warning includes action path for nested
  composites like actions/security/codeql-action/init.
- diff: gate String::replace with contains() so per-finding rewrite is
  allocation-free on the common path.
- main: route the remaining `git show` / `git rev-parse` calls through
  git_util::git_cmd() so the hooks-disabled invariant holds uniformly.
- BaseWorktree now lives inside a 0700 parent created via mkdtemp(3) on
  Unix; signal-cleanup and Drop both remove the parent after the
  worktree teardown. Non-Unix keeps the prior pid+nanos+counter scheme.
- cli::Args: diff_base stays the launcher --diff-base flag;
  diff_subcommand_base carries the `hasp diff <base>` positional.
- Trim audit-history prose from comments.
@electricapp
electricapp merged commit 358d628 into main May 28, 2026
5 checks passed
@electricapp
electricapp deleted the stack/04-diff branch May 28, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant