maintain: forbid unsafe and process-environment mutation

BN
3d ago
+9290-6597

What & why

This removes process-wide environment mutation and raw unsafe from the workspace, makes startup configuration explicit, and tightens credential/logging safety without changing the runner wire protocol.

The dependency prerequisite (e06c58e) is already isolated on main; this PR contains the remaining source, lint, CI, rule-fixture, documentation, and changelog changes.

Protocol surface

  • I confirm this change touches the runner protocol interface: NO

Required gates

  • cargo fmt --all --check
  • cargo check --locked --workspace --all-targets
  • cargo clippy --locked --workspace --all-targets -- -D warnings
  • Structural rule fixtures and strict scan pass on the verified tree
  • Runner-watch conformance passed on the verified tree
  • No new wire fields/events or protocol changes
  • No secrets, credentials, or deployment-specific paths introduced

What changed

  • Enforced workspace-wide unsafe_code = "forbid", including tests, benchmarks, and build scripts; restored lint inheritance for every workspace member.
  • Replaced production process-environment mutation with one-time startup resolution and typed configuration; tests use explicit inputs or child-process environment.
  • Replaced raw libc identity calls with safe nix APIs and socket activation with listenfd.
  • Removed unsafe VM/orchestrator/runner/observability/DAP/protocol test paths.
  • Added low-noise Clippy denies and ast-grep rules/fixtures for sensitive log fields, direct secret exposure in logs, TLS verification opt-outs, inline masking, and raw secret replacement.
  • Removed credential-bearing fields and signed URLs from logs and sanitized HTTP error rendering.
  • Updated CI to run full-workspace Clippy with -D warnings, structural rules, tests, and documentation/review guidance.

Verification performed

On macstudio after rebasing onto main:

  • cargo metadata --locked — pass
  • cargo fmt --all --check — pass
  • cargo check --locked --workspace --all-targets — pass
  • cargo clippy --locked --workspace --all-targets -- -D warnings — pass
  • git diff --check — pass
  • Verified tree: 2,806 tests passed, 3 ignored; remaining failures were Postgres control tests unable to create macOS System V shared-memory segments. The same failures reproduce on unmodified main; Linux CI remains the Postgres verification environment.
  • CLI startup, environment precedence, health/readiness, and authentication smoke checks — pass
  • Runner-watch conformance — pass
  • ast-grep fixtures and strict scan — pass

Checklist

  • Tests and rule fixtures updated where behavior changed
  • AGENTS.md, CONTRIBUTING.md, REVIEW.md, and CHANGELOG.md updated
  • No broad Clippy/pedantic or unwrap-ban policy added
  • No unexplained lint suppression added

Devin Review

Summary by cubic

Forbids unsafe code and process-environment mutation across the workspace, resolving startup configuration once into typed values and tightening credential logging with structural rules. No runner wire protocol changes.

Safety

  • unsafe_code = "forbid" covers every member including tests, benchmarks, and build scripts; std::env::set_var/remove_var are gone.
  • Raw libc calls became safe nix wrappers, and systemd socket activation uses listenfd; the now-unused libc and parking_lot declarations stay in place so Cargo.lock matches main, with the dependency removal shipping in a separate policy-only PR.
  • Startup reads the environment once into typed configuration (StartupConfig for the server); the CLI resolves the GitHub API base, engine, and workflow-scan token once per command boundary, and config helpers take explicit env values. The App-registration flag (PRELOOP_ENABLE_APP_REGISTRATION) resolves into typed config too, instead of being re-read on every request.
  • Tests pass values explicitly, build StartupConfig directly, or use Command::env on child processes; the manifest-registration unit tests were restored on the typed harness.

Logging and CI

  • Credential-bearing fields, signed URLs, and error-response bodies no longer reach logs.
  • ast-grep rules and fixtures cover sensitive log fields, secret exposure in logging macros, inline masking, raw secret replacement, and TLS verification opt-outs; the no-expose-in-loop rule was dropped because it could not distinguish legitimate per-secret use from the anti-pattern.
  • CI and just test-ci run workspace-wide Clippy with -D warnings, structural scans, and cargo check --all-targets --features test-support; every member inherits the lint set, and preloop-runner-server --lib also gates -D dead_code.

The branch is fully merged with main: backend-aware DNS, absolute mount paths, and injectable tool dirs are kept on top of the typed-config refactor; engine-hosted App setup, store and runner-bundle paths now receive startup values (public URL, credential store override, install wait) through StartupConfig/AppState instead of re-reading the process environment, and the golden/cleanup blocking closures own their path config. preloop run resolves its engine once per command, and tests use AppState::new_for_tests plus StartupConfig fixtures rather than the removed one-argument constructor.