feat: fix confirmed findings during scans with native agents - #1365

@greptile

@strix

Strix Security Review

All previously reported security findings have been resolved.

_{Updated for 9edb2ae.}

Reviewed by Strix
_{Re-run review · Configure security review settings}

Confidence Score: 5/5

[High risk] Adds automated fix generation and verification workflow to the scanning system.

The PR appears safe to merge based on the changes since the previous review.

What we checked:

• PR reviews start fix jobs: The runner checks for pr_review before it creates the fix manager, and the new test covers that choice on both new and resumed scans.

Summary

Strix now prepares fixes for confirmed source findings during scans, then checks each patch with a separate reviewer before making it available. Open-source source scans can create local branches from the scanned commit, and a new strix fix command runs the same preparation flow on its own.

• Saved findings can start native Fix agents in separate worktrees, including after a scan resumes.
• Independent review must approve the unchanged final patch before Strix marks it ready.
• CLI, report, and TUI updates show fix progress and prepared branch results.

_{Reviews (15) · Last reviewed commit: "fix: disable automatic fix agents for PR..."}

Comments Outside Diff

These findings could not be posted inline.

•  TUI setup skips automatic fixes strix/interface/cli_args.py:384 ▶

  When someone opens the TUI without a target and chooses a source target there, parse_arguments returns before setting the default for auto_fix. The TUI treats that unset value as False, so the scan starts no Fix agents and creates no fix branches. Set the default before this return.

Fixed in 49eca20. Every consumer path now resolves and containment-checks before touching the filesystem: anchor_location reads through _read_anchor_source (resolve + is_relative_to + UTF-8/decode guard), _apply_edits resolves each edit path and raises when it escapes the resolved workspace, and build_git_manifest only hashes files whose resolved path stays inside the workspace (directory expansion is containment-checked per file too).

Fixed in 49eca20. anchor_location no longer raises on unreadable or non-UTF-8 files (they anchor as missing and surface as candidate gaps), and _build_fix_candidate now catches any residual anchoring failure and keeps the un-anchored candidate with a known_gaps entry, so report persistence is never blocked by an unusable candidate.

Fixed in 49eca20. git status now runs with --untracked-files=all so new directories expand to per-file records, and any residual ?? directory entry is expanded recursively (with per-file workspace containment) so every new file gets a manifest entry and hash.

Fixed in 49eca20. _verify_source now requires a clean worktree (git status --porcelain must be empty) in addition to the matching HEAD commit, so a checkout carrying pre-existing uncommitted changes is rejected as stale rather than folded into the prepared manifest.

Fixed in 49eca20. Non-required checks that did not pass now contribute name: optional check <status> entries to gaps, which demotes the result to ready_with_gaps (or needs_review) so the failure is visible on the result instead of disappearing into a ready.

Fixed in 49eca20. The request's policy is now threaded into the default command runner: the subprocess environment is an allowlist (PATH/HOME/TEMP/VIRTUAL_ENV/etc.) plus only the variables named in credentials_allowed, and when network_allowed is false commands run under unshare in an empty network namespace — or are marked unavailable rather than executed when the platform cannot isolate egress.

Fixed in 49eca20. run_command no longer passes **os.environ: the subprocess environment is built from an explicit allowlist plus only the names listed in the request's credentials_allowed. The request's network_allowed is enforced too — when false, commands execute inside an unshare network namespace (probed once), and are returned unavailable instead of running when isolation isn't possible. prepare_fix binds both fields into the default runner for check and reproduction commands.

Fixed in 49eca20 — same change as the thread on prepare.py: both fields are now read by prepare_fix and bound into the default run_command (env allowlist + credentials_allowed passthrough; network_allowed=False runs commands under unshare or rejects them when isolation is unavailable).

Fixed in 49eca20. _apply_edits now resolves each edit path and raises ValueError when the resolved target is outside the resolved workspace, before any read or write. The same realpath containment was applied in anchor_location (escaping paths anchor as missing) and build_git_manifest (content is hashed only when the resolved path stays inside the workspace).

Fixed in 49eca20. anchor_location now reads through _read_anchor_source, which resolves the path, requires it to stay inside the resolved root, and returns missing for escapes or non-UTF-8/unreadable files — so the read primitive can no longer be redirected through a committed symlink.

This gate is the point of the PR, not a bug: SARIF fixes are automatically applicable, so emitting them for an unverified draft was the unsafe behavior being removed. prepare_fix is the engine's exported entrypoint — the preparation orchestrator runs it post-scan and writes the result back through update_vulnerability_report (fix_preparation became an updatable field in this PR), so a ready result does reach SARIF once preparation runs. In 49eca20 the gate also verifies candidate_digest against the stored fix_candidate and emits exactly the verified candidate's edits, so a ready result can never offer draft replacements that differ from what was actually verified.

@greptile

@strix-security

Fixed in 2b413da. _refresh_fix_candidate now supersedes the recorded preparation whenever candidate-affecting fields change and the report carries a fix candidate or preparation — even when the revised fields can't rebuild one (e.g. ./file.py locations). The stale ready result can no longer outlive the locations it was prepared against.

Fixed in 2b413da. FixPreparationResultV1 now carries the candidate preparation actually verified (result.candidate, with candidate_digest binding it), so SARIF emits the anchor-corrected candidate's edits directly instead of digest-matching the stored draft against the re-anchored one. Stored fix_candidate remains only a fallback for write-backs that predate this field.

@greptile

Fixed in be1c2e4. Right after _apply_edits, preparation now hashes each edited file, and compares that set against the final manifest: any extra file, revised edit, or reverted edit yields the gap "The verified change extends beyond the recorded draft edits." and demotes the result to ready_with_gaps — which the SARIF gate already refuses to auto-apply. So a one-click fix can now only ever equal the change set preparation verified; anything wider stays visible via the gap and the manifest.

@strix

@greptile

@greptile

@strix

@strix

@greptile

@greptile

@greptile

@greptile

@strix

@greptile