diff --git a/Cargo.lock b/Cargo.lock index e4c292ade..a4fc855ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1573,8 +1573,8 @@ dependencies = [ "fabro-http", "fabro-model", "fabro-oauth", + "fabro-redact", "fabro-static", - "fabro-util", "fabro-vault", "httpmock", "serde", @@ -1640,6 +1640,7 @@ dependencies = [ "fabro-model", "fabro-oauth", "fabro-proc", + "fabro-redact", "fabro-retro", "fabro-sandbox", "fabro-server", @@ -1786,10 +1787,10 @@ dependencies = [ "chrono", "fabro-http", "fabro-macros", + "fabro-redact", "fabro-static", "fabro-test", "fabro-types", - "fabro-util", "jsonwebtoken", "serde", "serde_json", @@ -1888,6 +1889,7 @@ dependencies = [ "fabro-http", "fabro-macros", "fabro-model", + "fabro-redact", "fabro-static", "fabro-test", "fabro-util", @@ -1950,6 +1952,7 @@ dependencies = [ "axum", "base64", "fabro-http", + "fabro-redact", "fabro-static", "fabro-test", "fabro-util", @@ -1972,6 +1975,22 @@ dependencies = [ "tempfile", ] +[[package]] +name = "fabro-redact" +version = "0.213.0-nightly.0" +dependencies = [ + "aho-corasick", + "ref-cast", + "regex", + "serde", + "serde_json", + "thiserror 2.0.18", + "toml 0.8.23", + "tracing", + "tracing-subscriber", + "url", +] + [[package]] name = "fabro-retro" version = "0.213.0-nightly.0" @@ -2050,6 +2069,7 @@ dependencies = [ "fabro-llm", "fabro-model", "fabro-proc", + "fabro-redact", "fabro-retro", "fabro-sandbox", "fabro-slack", @@ -2257,7 +2277,6 @@ dependencies = [ name = "fabro-util" version = "0.213.0-nightly.0" dependencies = [ - "aho-corasick", "anyhow", "console 0.15.11", "dirs", @@ -2265,18 +2284,13 @@ dependencies = [ "insta", "open", "rand 0.9.4", - "ref-cast", - "regex", "serde", "serde_json", "tempfile", "termimad", - "thiserror 2.0.18", "tokio", - "toml 0.8.23", "tracing", "tracing-subscriber", - "url", ] [[package]] @@ -2326,6 +2340,7 @@ dependencies = [ "fabro-macros", "fabro-mcp", "fabro-model", + "fabro-redact", "fabro-retro", "fabro-sandbox", "fabro-static", diff --git a/Cargo.toml b/Cargo.toml index ec67b0515..614d82766 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -79,6 +79,7 @@ rust-embed = "8" percent-encoding = "2" minijinja = "2" fabro-http = { path = "lib/crates/fabro-http" } +fabro-redact = { path = "lib/crates/fabro-redact" } fabro-static = { path = "lib/crates/fabro-static" } graphviz-sys = { git = "https://github.com/fabro-sh/graphviz-sys" } ref-cast = "1" diff --git a/clippy.toml b/clippy.toml index 066253a39..406f78c74 100644 --- a/clippy.toml +++ b/clippy.toml @@ -41,6 +41,6 @@ disallowed-types = [ { path = "std::net::TcpStream", reason = "Blocking socket; prefer tokio::net::TcpStream on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" }, { path = "std::net::TcpListener", reason = "Blocking accept; prefer tokio::net::TcpListener on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" }, { path = "std::net::UdpSocket", reason = "Blocking recv/send; prefer tokio::net::UdpSocket on Tokio paths. Document intentional sync I/O with #[expect(clippy::disallowed_types, reason = \"...\")]" }, - { path = "url::Url", reason = "Use fabro_util::redact::DisplaySafeUrl at logging/error boundaries for URLs that may carry credentials; document intentional raw URL transit with #[expect(clippy::disallowed_types, reason = \"...\")]", allow-invalid = true }, - { path = "reqwest::Url", reason = "Use fabro_util::redact::DisplaySafeUrl at logging/error boundaries for URLs that may carry credentials; document intentional raw URL transit with #[expect(clippy::disallowed_types, reason = \"...\")]", allow-invalid = true }, + { path = "url::Url", reason = "Use fabro_redact::DisplaySafeUrl at logging/error boundaries for URLs that may carry credentials; document intentional raw URL transit with #[expect(clippy::disallowed_types, reason = \"...\")]", allow-invalid = true }, + { path = "reqwest::Url", reason = "Use fabro_redact::DisplaySafeUrl at logging/error boundaries for URLs that may carry credentials; document intentional raw URL transit with #[expect(clippy::disallowed_types, reason = \"...\")]", allow-invalid = true }, ] diff --git a/docs-internal/logging-strategy.md b/docs-internal/logging-strategy.md index 3e035b1db..07a2a099c 100644 --- a/docs-internal/logging-strategy.md +++ b/docs-internal/logging-strategy.md @@ -194,4 +194,4 @@ Some field values carry real or latent sensitivity and must not appear in `traci These prohibitions apply to every level (ERROR through TRACE). If an error path genuinely needs raw output for triage, route it through an authenticated support channel — not the default tracing subscriber. -For URLs that may carry credentials, log `fabro_util::redact::DisplaySafeUrl` or a string produced by `DisplaySafeUrl::redacted_string()`. Its `Display` and `Debug` forms redact userinfo plus these query keys case-insensitively: `token`, `install_token`, `access_token`, `refresh_token`, `api_key`, `apikey`, `code`, `state`, `password`, `secret`, and `key`. Raw URL strings stay reserved for wire transit, subprocess arguments, redirects, and persistence. +For URLs that may carry credentials, log `fabro_redact::DisplaySafeUrl` or a string produced by `DisplaySafeUrl::redacted_string()`. Its `Display` and `Debug` forms redact userinfo plus these query keys case-insensitively: `token`, `install_token`, `access_token`, `refresh_token`, `api_key`, `apikey`, `code`, `state`, `password`, `secret`, and `key`. Raw URL strings stay reserved for wire transit, subprocess arguments, redirects, and persistence. diff --git a/docs/plans/2026-04-19-002-feat-run-files-changed-tab-plan.md b/docs/plans/2026-04-19-002-feat-run-files-changed-tab-plan.md index 8db8c72e4..36e46a2de 100644 --- a/docs/plans/2026-04-19-002-feat-run-files-changed-tab-plan.md +++ b/docs/plans/2026-04-19-002-feat-run-files-changed-tab-plan.md @@ -134,7 +134,7 @@ Carried from origin + new measurable targets: - **Sandbox reconnect**: `lib/crates/fabro-sandbox/src/reconnect.rs:17-60` (dispatches on `provider` string; only Local and Daytona are compiled into the server — `fabro-server/Cargo.toml:25` enables only `features = ["daytona"]`, so Docker runs would 409 at reconnect). `reconnect_run_sandbox` in `lib/crates/fabro-server/src/server.rs:5797-5806` maps all reconnect errors to `StatusCode::CONFLICT`. - **Sandbox git helpers**: `lib/crates/fabro-workflow/src/sandbox_git.rs`. `GIT_REMOTE` (line 23), `shell_quote` (line 26), `git_diff` at line 185 is `pub(crate)` with a 30 s timeout and returns an unbounded string — not fit for the new endpoint. A sibling machine-readable helper is needed. - **`RunProjection.final_patch`**: `lib/crates/fabro-store/src/run_state.rs:35`, populated at `lib/crates/fabro-workflow/src/lifecycle/git.rs:303-332` via `on_run_end` when `outcome.status == Success || PartialSuccess`. Extending to Failed is the lifecycle change in Unit 2. -- **Redaction**: `lib/crates/fabro-util/src/redact/` is gitleaks+entropy at the content level. No path-denylist exists anywhere — Unit 8 is greenfield for that. +- **Redaction**: `lib/crates/fabro-redact/src/` is gitleaks+entropy at the content level. No path-denylist exists anywhere — Unit 8 is greenfield for that. - **Frontend API helpers**: `apps/fabro-web/app/api.ts:41-47` (`apiJson`), `:115-130` (`apiJsonOrNull` — handles 404 and 501 gracefully — use this for the new route). - **Route conventions**: loaders use `({ request, params })`; `export const handle = { wide: true }` toggles wide layout; data flows through `withRouteModule` wrapper (`router.tsx:42-49`). - **Tab visibility**: `apps/fabro-web/app/routes/run-detail.tsx:14` has `{ name: "Files Changed", path: "/files", broken: true }`; `:68` filters with `!t.broken`. Removing the flag makes the tab visible. @@ -182,7 +182,7 @@ There is no `docs/solutions/` directory in this repo; `docs/plans/` and `docs/br | **Response shape**: reuse `PaginatedRunFileList` name; introduce `RunFilesMeta` replacing `PaginationMeta` on this endpoint. | Keeps the existing schema name (already in the spec and the TS client as a type alias). Adds `truncated`, `total_changed`, optional `degraded`, optional `patch` without touching shared `PaginationMeta`. Non-breaking for consumers of `PaginationMeta` on other endpoints. | | **No cursor pagination; single top-level `truncated` + `total_changed`.** | Natural-bound list with a hard 200-file cap. Follows `ListResponse` precedent where "paginated" just means "bounded". Adding a cursor is deferred until a consumer needs it. | | **`change_kind` is optional.** | Resolves the R19/R23 back-compat tension: existing progenitor/openapi-generator clients can ignore it; v1 clients that know about it can skip hunk inspection. Kept as additive-only metadata. | -| **Denylist in server code; content-based scanning out of scope.** | `fabro-util::redact` is content-level; the brainstorm explicitly confines R31 to path-based skipping. Matches the "authz gate + `git diff` parity" framing in R30. Config-driven denylist can come later. | +| **Denylist in server code; content-based scanning out of scope.** | `fabro-redact` is content-level; the brainstorm explicitly confines R31 to path-based skipping. Matches the "authz gate + `git diff` parity" framing in R30. Config-driven denylist can come later. | | **Request coalescing, not queuing.** | Concurrent callers for the same run share one in-flight materialization via a singleflight map on `AppState`. Avoids redundant git work, prevents queue-exhaustion DoS, and is simpler than a 429-with-retry policy. | | **Use `apiJsonOrNull` on the web loader.** | Handles 404 and 501 gracefully, so dev environments without the new route (or demo without the new demo stub) render R4(c) empty state instead of hitting `RootErrorBoundary`. Mirrors the precedent in `run-stages.tsx:118` and `run-overview.tsx:16`. | @@ -669,7 +669,7 @@ flowchart TB - **Extend `docs-internal/logging-strategy.md`** with the new prohibited-field entries (`diff_contents`, `file_path` for changed-file paths specifically, `git_stderr`) so future endpoints inherit the rule. **Patterns to follow:** -- `lib/crates/fabro-util/src/redact/jsonl.rs:21` — the existing redaction field skiplist, as the conceptual ancestor (but content-level, not path-level). +- `lib/crates/fabro-redact/src/jsonl.rs:21` — the existing redaction field skiplist, as the conceptual ancestor (but content-level, not path-level). - `AGENTS.md` shell-quoting rule for any shell-adjacent code. **Test scenarios:** diff --git a/docs/plans/2026-04-19-003-feat-cli-auth-login-plan.md b/docs/plans/2026-04-19-003-feat-cli-auth-login-plan.md index 32e77523c..a870d8983 100644 --- a/docs/plans/2026-04-19-003-feat-cli-auth-login-plan.md +++ b/docs/plans/2026-04-19-003-feat-cli-auth-login-plan.md @@ -916,14 +916,14 @@ Organized into five phases. Early phases stand alone (no behavior changes). Late **Logging invariant (key technical decision):** - Never log raw bearer values, raw auth codes, or `code_verifier`. -- Never log raw `user_agent` strings — they're attacker-controlled and can carry ANSI escape sequences (terminal injection for operators tail-ing logs) or newlines (log-splitting). Log a `user_agent_fingerprint = hex(sha256(user_agent)[..8])` instead — stable for correlation, inert as payload. SHA-256 is already in the workspace; no new dependency. If the raw UA is needed for support debugging, route it through `fabro_util::redact` which strips control characters. +- Never log raw `user_agent` strings — they're attacker-controlled and can carry ANSI escape sequences (terminal injection for operators tail-ing logs) or newlines (log-splitting). Log a `user_agent_fingerprint = hex(sha256(user_agent)[..8])` instead — stable for correlation, inert as payload. SHA-256 is already in the workspace; no new dependency. If the raw UA is needed for support debugging, route it through `fabro-redact` which strips control characters. - User email is logged at INFO only on `/token` (login event) and `/logout` (explicit exit event); not on `/refresh` rotations. - Enforced by review at implementation time and documented in the module's top-level doc comment. **Patterns to follow:** - Existing handler signature patterns for state/extension extraction in `server.rs:2771` (`list_runs`) and `web_auth.rs:285` (`login_github`). - Generate 32-byte secrets via `OsRng::fill_bytes`, encode with `base64::engine::general_purpose::URL_SAFE_NO_PAD`. -- Existing `fabro_util::redact` patterns for any case where bearer-adjacent data must appear in logs. +- Existing `fabro-redact` patterns for any case where bearer-adjacent data must appear in logs. **Test scenarios:** - Happy path (/token): valid code + verifier → 200 with access+refresh; refresh row exists in store. diff --git a/docs/plans/2026-04-23-003-refactor-llm-client-resolution-and-run-services-plan.md b/docs/plans/2026-04-23-003-refactor-llm-client-resolution-and-run-services-plan.md index 83c5c0fbe..cdbde5793 100644 --- a/docs/plans/2026-04-23-003-refactor-llm-client-resolution-and-run-services-plan.md +++ b/docs/plans/2026-04-23-003-refactor-llm-client-resolution-and-run-services-plan.md @@ -311,7 +311,7 @@ Phase data flow: - `EnvCredentialSource::new()` reads env for each provider in `Provider::ALL`, emitting `ApiCredential`s. `auth_issues` is always empty. Replaces the body of today's `Client::from_env`. - `Client::from_source(source: &dyn CredentialSource) -> Result, Error>` — calls `source.resolve()`, discards `auth_issues`, calls `Client::from_credentials`, wraps in `Arc`. Callers that need diagnostics call `source.resolve()` directly. - Move `auth_issue_message` helper (today duplicated in `fabro-workflow/handler/llm/api.rs:82` and `fabro-server/server_secrets.rs:127`) to `fabro-auth` alongside the trait. - - `ApiCredential` must have a redacting `Debug` impl (verify/add using `fabro-util::redaction`). Add unit test asserting `format!("{:?}", credential)` does not echo key material. + - `ApiCredential` must have a redacting `Debug` impl (verify/add using `fabro-redact`). Add unit test asserting `format!("{:?}", credential)` does not echo key material. **Patterns to follow:** - `CredentialFallback` trait shape in `docs/plans/2026-04-20-002-refactor-extract-fabro-client-crate-plan.md`. diff --git a/docs/plans/2026-04-24-001-refactor-adopt-uv-patterns-plan.md b/docs/plans/2026-04-24-001-refactor-adopt-uv-patterns-plan.md index 762fb844a..52068c597 100644 --- a/docs/plans/2026-04-24-001-refactor-adopt-uv-patterns-plan.md +++ b/docs/plans/2026-04-24-001-refactor-adopt-uv-patterns-plan.md @@ -23,7 +23,7 @@ Six uv patterns, ordered by risk/value so each phase is individually shippable: | # | Phase | New crate(s) | Scope | |---|---|---|---| | 1 | `fabro-static` env var registry | `fabro-static` | Replace ~113 `env::var` string literals and 7 clap `env =` sites with constants on an `EnvVars` struct | -| 2 | `DisplaySafeUrl` newtype | (extend `fabro-util::redact`) | Add credential-redacting `Url` wrapper; migrate high-risk URL logging call sites | +| 2 | `DisplaySafeUrl` newtype | `fabro-redact` | Add credential-redacting `Url` wrapper; migrate high-risk URL logging call sites | | 3 | Expanded snapshot helpers | (extend `fabro-test`) | Lift `fabro_json_snapshot!` into `fabro-test`; add value-shaped wrapper parallel to existing `fabro_snapshot!` | | 4 | `miette` CLI diagnostics | (extend `fabro-cli`) | Wrap CLI root error with `miette::Diagnostic` for styled chained errors and help footers while preserving existing exit-class hint logic | | 5 | `cargo dev` unified CLI | `fabro-dev` | New binary crate replacing `bin/dev/*.sh` and `scripts/*-spa*.sh`; add `dev = "run -p fabro-dev --"` alias | @@ -82,8 +82,8 @@ None of these is a crisis today. Each is a cheap fix when done deliberately, and ### Relevant Code and Patterns **Existing fabro infrastructure to reuse:** -- `lib/crates/fabro-util/src/redact/` — existing redaction home. `redact_string`, `redact_json_value`, and `redact_jsonl_line` cover generic secret scanning with entropy and gitleaks rules; Phase 2 adds deterministic URL display redaction here. -- `lib/crates/fabro-http/src/lib.rs:12` — re-exports `url::Url` as `fabro_http::Url`. Already has `#![allow(clippy::disallowed_methods, clippy::disallowed_types)]`, but URL redaction belongs in `fabro-util::redact` rather than the HTTP facade. +- `lib/crates/fabro-redact/src/` — redaction home. `redact_string`, `redact_json_value`, and `redact_jsonl_line` cover generic secret scanning with entropy and gitleaks rules; Phase 2 adds deterministic URL display redaction here. +- `lib/crates/fabro-http/src/lib.rs:12` — re-exports `url::Url` as `fabro_http::Url`. Already has `#![allow(clippy::disallowed_methods, clippy::disallowed_types)]`, but URL redaction belongs in `fabro-redact` rather than the HTTP facade. - `lib/crates/fabro-util/src/env.rs` — `Env` trait with `SystemEnv` / `TestEnv` impls. Orthogonal to the name registry; keep it. The registry is about *names*; the trait is about *injection*. - `lib/crates/fabro-macros/src/lib.rs` — proc-macro crate with `Combine` derive and `e2e_test` attr already in place. Deps (`syn`, `quote`, `proc-macro2`) are already wired; `OptionsMetadata` macro plumbing slots in cleanly. The runtime trait/data model cannot live in this proc-macro crate; Phase 6 must add a normal crate (`fabro-options-metadata`) parallel to uv's `uv-options-metadata`. - `lib/crates/fabro-test/src/lib.rs:61` — `INSTA_FILTERS` with 10 pre-baked filters. Line 1716: `fabro_snapshot!` macro. Line 1214: `TestContext::add_filter`. Good foundation for JSON variant. @@ -150,9 +150,9 @@ No existing learnings on miette, cargo-dev tooling, or CLI docs drift — those ## Key Technical Decisions - **New `fabro-static` crate rather than adding `EnvVars` to `fabro-util`.** Rationale: `fabro-util` is a grab-bag; `fabro-static` mirrors uv's narrow-purpose crate and reads as a first-class convention. Leaf crates can depend on `fabro-static` without pulling in `fabro-util`'s broader surface. -- **`DisplaySafeUrl` lives in `fabro-util::redact` rather than `fabro-http` or a new crate.** Rationale: Fabro already has a shared redaction home for generic string/JSONL secret scanning, and URL display redaction is the same ownership domain. `fabro-http` stays a raw HTTP facade; public API DTOs remain raw `String` / `url::Url` unless a separate DTO audit says otherwise. +- **`DisplaySafeUrl` lives in `fabro-redact` rather than `fabro-http` or `fabro-util`.** Rationale: Fabro already had shared redaction code for generic string/JSONL secret scanning, and URL display redaction is the same ownership domain. Moving that surface into a narrow crate keeps `fabro-http` a raw HTTP facade and keeps `fabro-util` from owning credential logic. Public API DTOs remain raw `String` / `url::Url` unless a separate DTO audit says otherwise. - **`Debug` delegates to `Display` (fabro diverges from uv).** uv's `Debug` impl is a `debug_struct` with separate `scheme`/`username`/`password`/`host`/`port`/`path`/`query`/`fragment` fields — redacting only username/password and leaving query/path/fragment raw. That is unsafe for fabro because our token-bearing URLs put tokens in query strings (`?token=...`), not userinfo. Fabro's `Display` redacts both userinfo and query-string keys from an allowlist; `Debug` delegates to `Display` so `?url` in `tracing::debug!(?url, ...)` does not leak tokens. This is the single most important design choice in Phase 2; get it wrong and the whole phase is security theater. -- **`DisplaySafeUrl` redacts query-string keys from an allowlist, not just userinfo.** uv's implementation only redacts the password in the URL authority. Fabro needs more: install tokens, CSRF `state` tokens, API keys, auth codes, and access tokens all ride in query strings. The allowlist at minimum: `token`, `install_token`, `access_token`, `refresh_token`, `api_key`, `apikey`, `code`, `state`, `password`, `secret`, `key`. Allowlist lives in `fabro-util::redact` and is documented in `docs-internal/logging-strategy.md`. +- **`DisplaySafeUrl` redacts query-string keys from an allowlist, not just userinfo.** uv's implementation only redacts the password in the URL authority. Fabro needs more: install tokens, CSRF `state` tokens, API keys, auth codes, and access tokens all ride in query strings. The allowlist at minimum: `token`, `install_token`, `access_token`, `refresh_token`, `api_key`, `apikey`, `code`, `state`, `password`, `secret`, `key`. Allowlist lives in `fabro-redact` and is documented in `docs-internal/logging-strategy.md`. - **Serialization must be redacted when it exists, but a blanket impl is optional.** The "display-only" redaction story is insufficient for DTO/output paths: if a JSON response contains a redacted URL type, the response body must render redacted. However, implementing `Serialize` directly on `DisplaySafeUrl` can silently persist `****` if the type leaks into config/cache/storage structs. Unit 2.1 must choose between (a) a blanket redacted `Serialize` impl plus strong persistence bans/tests, or (b) a separate output-only wrapper for JSON/schema boundaries. In both designs, no serializable redacted URL type ever serializes raw credentials. - **Scope of Phase 2 migration is narrowed to credential-bearing paths only.** The research surfaced ~15 URL-logging sites; many of them (`fabro-server/src/install.rs:1454`, `github_webhooks.rs:62`, `serve.rs:331`, `canonical_origin.rs:2`) log URLs that never carry credentials. Migrating them to `DisplaySafeUrl` is churn without security benefit. Phase 2 covers only sites that construct, log, or serialize URLs that can carry tokens; non-credentialed URL handling stays on `url::Url`. - **`DisplaySafeUrl` is a display-layer adapter, not a URL type.** The type exists at exactly three kinds of boundaries: @@ -174,23 +174,23 @@ No existing learnings on miette, cargo-dev tooling, or CLI docs drift — those - **`OptionsMetadata` lands last and is split like uv.** Rationale: the runtime metadata model is a normal crate (`fabro-options-metadata`) with `OptionsMetadata`, `Visit`, `OptionField`, and `OptionSet`; `fabro-macros` only generates impls for that trait. The proc macro is the biggest surface and depends on clap/settings structs being stable. Generating docs from it also needs the `fabro-dev` host crate to exist. Keeping it last also means cli.mdx drift catches up in one PR rather than churning twice. - **Clippy enforcement: workspace-wide bans with crate-level opt-outs.** `clippy.toml` is workspace-global — there is no per-crate include/exclude mechanism. Each phase ships bans with this model: - **Workspace-wide ban** in `clippy.toml` (with `allow-invalid = true` so the ban survives even when the facade crate isn't in scope). - - **Crate-level `#![allow(...)]`** only at the root of crates whose entire purpose legitimately owns/facades the banned symbol (`fabro-static`, `fabro-http`, test-support crates). Mixed crates that contain both sensitive and non-sensitive paths (especially `fabro-server`) do **not** get root-level allows; they use module-level allows or narrow `#[expect]` at the raw-use site. In `fabro-util`, the allowance is scoped to the `redact::safe_url` owner module. + - **Crate-level `#![allow(...)]`** only at the root of crates whose entire purpose legitimately owns/facades the banned symbol (`fabro-static`, `fabro-http`, test-support crates). Mixed crates that contain both sensitive and non-sensitive paths (especially `fabro-server`) do **not** get root-level allows; they use module-level allows or narrow `#[expect]` at the raw-use site. In `fabro-redact`, the allowance is scoped to the `safe_url` owner module. - **`#[expect(..., reason = "...")]`** at individual call sites within credential-handling crates where a raw reference is unavoidable. - Phase 1 adds `disallowed-methods` for `std::env::var`/`var_os` workspace-wide. `fabro-static` and every `build.rs` carry `#![allow(clippy::disallowed_methods)]`; dynamic resolver/facade paths such as `fabro-util::env::SystemEnv`, interpolation closures, and subprocess env allowlists carry narrow `#[expect]` or module-level allowances with reasons. - - Phase 2 adds `disallowed-types` for raw `url::Url`/`reqwest::Url` workspace-wide once migration is complete. `fabro-util::redact::safe_url` (owner), `fabro-http` (reqwest facade), and test-only support can use scoped allowances; mixed production crates use module/call-site allowances so credential-bearing modules remain protected. Phase 2 also adds a workspace-wide `disallowed-macros` or regex-CI check against inline `format!("https://{}:{}@{}", ...)` construction (no legitimate callers). + - Phase 2 adds `disallowed-types` for raw `url::Url`/`reqwest::Url` workspace-wide once migration is complete. `fabro-redact::safe_url` (owner), `fabro-http` (reqwest facade), and test-only support can use scoped allowances; mixed production crates use module/call-site allowances so credential-bearing modules remain protected. Phase 2 also adds a workspace-wide `disallowed-macros` or regex-CI check against inline `format!("https://{}:{}@{}", ...)` construction (no legitimate callers). - The purpose of the bans is regression prevention, not migration gating. If a crate accumulates many `#[expect]`s, that's a signal to keep migrating — not to bake the exceptions in. ## Open Questions ### Resolved During Planning -- **Where does `DisplaySafeUrl` live?** → `fabro-util::redact`, next to the existing generic redaction helpers. +- **Where does `DisplaySafeUrl` live?** → `fabro-redact`, next to the generic redaction helpers. - **Where does `EnvVars` live?** → New `fabro-static` crate. - **Where does `OptionsMetadata` runtime metadata live?** → New `fabro-options-metadata` crate. `fabro-macros` only owns the derive macro and emits impls against that normal crate. - **Miette full swap or wrap?** → Wrap-in-main (option b). - **Does fabro already have a snapshot-filter macro?** → Yes, `fabro_snapshot!` in `fabro-test`. Phase 3 adds a JSON/value variant, not a greenfield macro. - **Does `Debug` delegate to `Display`, matching uv?** → No. uv's `Debug` leaves query/path raw; fabro delegates `Debug` to `Display`. Query-string tokens are redacted in both. This is a deliberate divergence documented in Key Technical Decisions. -- **Does `DisplaySafeUrl` redact query-string keys?** → Yes, from an allowlist maintained in `fabro-util::redact`. uv's impl does not. +- **Does `DisplaySafeUrl` redact query-string keys?** → Yes, from an allowlist maintained in `fabro-redact`. uv's impl does not. - **Does a serializable redacted URL produce the raw URL (uv parity) or the redacted form?** → Redacted form. Whether that is a blanket `Serialize` impl on `DisplaySafeUrl` or a separate output-only wrapper is deferred to Unit 2.1; either way, callers needing raw-on-wire data use `.as_raw_url()` / `.raw_string()` / `Deref`, not serialization. - **Do non-credentialed URL logging sites migrate to `DisplaySafeUrl`?** → No. Migration scope is narrowed to credential-bearing paths. Sites like `fabro-server/src/install.rs:1454` (public webhook URL logging) stay on `url::Url`. - **Do the three existing partial env var consts get per-crate decisions?** → No. Two have cross-crate importers but migration is uniform. Delete all three; update importers to reference `fabro_static::EnvVars::*` directly. @@ -217,7 +217,7 @@ No existing learnings on miette, cargo-dev tooling, or CLI docs drift — those lib/crates/ ├── fabro-static/ (NEW — Phase 1) │ └── EnvVars struct, all FABRO_* and upstream constants -├── fabro-util/src/redact/ (MODIFIED — Phase 2) +├── fabro-redact/ (NEW — Phase 2) │ └── DisplaySafeUrl, DisplaySafeUrlError, ref-cast impls ├── fabro-test/ (MODIFIED — Phase 3) │ └── add fabro_json_snapshot! + JSON filter set @@ -344,21 +344,22 @@ Solid arrows are real dependencies. Dashed lines show phases that are independen --- -### Phase 2 — fabro-util DisplaySafeUrl +### Phase 2 — fabro-redact DisplaySafeUrl -- [x] **Unit 2.1: Add `DisplaySafeUrl` to `fabro-util::redact`** +- [x] **Unit 2.1: Create `fabro-redact` crate with redaction helpers and `DisplaySafeUrl`** -**Goal:** Extend the existing redaction module with a credential-redacting `Url` wrapper. +**Goal:** Move existing redaction helpers into a narrow redaction crate and add a credential-redacting `Url` wrapper. **Requirements:** R2, R7. **Dependencies:** None (independent of Phase 1; can interleave). **Files:** -- Create: `lib/crates/fabro-util/src/redact/safe_url.rs` -- Modify: `lib/crates/fabro-util/src/redact/mod.rs` -- Modify: `lib/crates/fabro-util/Cargo.toml` and root `Cargo.toml` (add `ref-cast` and `url` workspace deps if not present) -- Test: `lib/crates/fabro-util/src/redact/safe_url.rs` (inline unit tests mirroring uv's) +- Create: `lib/crates/fabro-redact/Cargo.toml` +- Move: `lib/crates/fabro-util/src/redact/*` to `lib/crates/fabro-redact/src/` +- Move: `lib/crates/fabro-util/build.rs` and `lib/crates/fabro-util/data/gitleaks.toml` to `lib/crates/fabro-redact/` +- Modify: root `Cargo.toml` (add `fabro-redact`, `ref-cast`, and `url` workspace deps if not present) +- Test: `lib/crates/fabro-redact/src/safe_url.rs` (inline unit tests mirroring uv's) **Approach:** - Use `/Users/bhelmkamp/p/astral-sh/uv/crates/uv-redacted/src/lib.rs` as a starting template — structure, `#[repr(transparent)]`, `RefCast`, `DisplaySafeUrlError::AmbiguousAuthority`, `has_credential_like_pattern`. But fabro **diverges in three material ways** (see Key Technical Decisions): @@ -397,8 +398,8 @@ Solid arrows are real dependencies. Dashed lines show phases that are independen - Integration (schemars, if a serializable redacted-output type exists): `schemars::schema_for!(DisplaySafeUrl)` or the output wrapper produces a schema with `type: string, format: uri` (transparent). **Verification:** -- `cargo test -p fabro-util safe_url` passes. -- `cargo doc -p fabro-util --no-deps` produces docs without warnings. +- `cargo test -p fabro-redact` passes. +- `cargo doc -p fabro-redact --no-deps` produces docs without warnings. --- @@ -466,7 +467,7 @@ Solid arrows are real dependencies. Dashed lines show phases that are independen **Verification:** - `rg 'format!\("https?://[^"]*:[^"]*@' lib/crates/` returns zero matches (no more inline token URL construction). - Tracing-capture integration tests in `fabro-github`, `fabro-oauth`, `fabro-server` (install flow) pass and assert no token substring. -- `clippy.toml` gains workspace-wide `disallowed-types` bans on `url::Url` and `reqwest::Url`. `fabro-util::redact::safe_url` (owner), `fabro-http` (reqwest facade), and test-support code can carry scoped `#[allow(clippy::disallowed_types)]`. Mixed production crates such as `fabro-server` use module-level allowances or per-site `#[expect(clippy::disallowed_types, reason = "...")]` so sensitive modules are still protected. +- `clippy.toml` gains workspace-wide `disallowed-types` bans on `url::Url` and `reqwest::Url`. `fabro-redact::safe_url` (owner), `fabro-http` (reqwest facade), and test-support code can carry scoped `#[allow(clippy::disallowed_types)]`. Mixed production crates such as `fabro-server` use module-level allowances or per-site `#[expect(clippy::disallowed_types, reason = "...")]` so sensitive modules are still protected. - Existing `cargo nextest run` passes. --- @@ -897,7 +898,7 @@ impl fabro_options_metadata::OptionsMetadata for RunArgs { ## System-Wide Impact -- **Interaction graph:** `fabro-static` and `fabro-options-metadata` are new leaf/runtime crates with no existing consumers; adding them to workspace members is safe. Phase 2 extends `fabro-util::redact`, which is already the shared redaction surface. `fabro-cli` gains a direct dep on `miette` for the CLI boundary and an exposed metadata surface for docs generation. `fabro-dev` depends on that exposed clap/metadata surface at generator time (not in the shipped `fabro` binary runtime path). +- **Interaction graph:** `fabro-static`, `fabro-redact`, and `fabro-options-metadata` are new leaf/runtime crates with no existing consumers; adding them to workspace members is safe. `fabro-redact` owns the shared redaction surface previously housed under `fabro-util::redact`. `fabro-cli` gains a direct dep on `miette` for the CLI boundary and an exposed metadata surface for docs generation. `fabro-dev` depends on that exposed clap/metadata surface at generator time (not in the shipped `fabro` binary runtime path). - **Error propagation:** Phase 4 wraps at the `main` boundary; library error types unchanged. `anyhow::Error::chain()` still flows through `.context(...)` calls; only the final render changes. - **State lifecycle risks:** None from Phases 1, 3, 4, 5, 6 (pure refactors). Phase 2's redaction reaches Display and Debug, plus any explicitly serializable redacted-output type. Callers that need the raw URL for wire-level operations (reqwest, git CLI, subprocess args, HTTP `Location:` headers, stderr prompts the user acts on) must use `.as_raw_url()` / `.raw_string()`. `url.to_string()` on a `DisplaySafeUrl` returns the *redacted* form — an intentional trap that's neutralized by the Key Technical Decisions mitigation stack (explicit methods, clippy deny on `::to_string`, `disallowed-types` ban, and per-call-site tests in sandbox/install paths). - **API surface parity:** `fabro --help` output stays identical through Phase 1 (clap consts vs literals produce the same help). Phase 6 may reshape `cli.mdx` rendering but not `--help`. Phase 2 does NOT touch public API DTOs — `avatar_url`, `user_url`, and any OpenAPI-generated schemas remain `string`/`url::Url`. @@ -915,7 +916,7 @@ impl fabro_options_metadata::OptionsMetadata for RunArgs { |------|------------| | Phase 1 churn (~113 call sites, 20 crates) produces a large diff and high merge-conflict probability against in-flight work | Ship Phase 1 alone in a dedicated PR. Coordinate timing with any other heavy refactors. Stage per-crate commits inside the PR for reviewability. | | Phase 2 migration misses a credential-bearing URL path; a leak survives | Tracing-capture integration tests in `fabro-github`, `fabro-oauth`, and `fabro-server` assert no token substring. Phase 2 also ships a workspace `clippy.toml` `disallowed-types` ban on raw `url::Url`/`reqwest::Url`; mixed crates use module/call-site expectations rather than root-level allows, and a `format!("https://...:...@")` guard catches inline token URL construction. | -| Query-string-token URLs (`?token=...`, `?state=...`) not covered by uv's DisplaySafeUrl design | Fabro diverges from uv with an allowlist-based query-key redactor baked into `Display`. Allowlist lives in `fabro-util::redact` and is documented in `docs-internal/logging-strategy.md`. | +| Query-string-token URLs (`?token=...`, `?state=...`) not covered by uv's DisplaySafeUrl design | Fabro diverges from uv with an allowlist-based query-key redactor baked into `Display`. Allowlist lives in `fabro-redact` and is documented in `docs-internal/logging-strategy.md`. | | `Debug` impl diverging from uv breaks snapshot tests that asserted against `?url` or struct-shaped debug output | Audit existing snapshots before Phase 2 lands; `fabro_snapshot!` default filters can normalize URL rendering in snapshots that should be agnostic to the redaction format. | | `shell_quote` / `.to_string()` trap: `url.to_string()` on a `DisplaySafeUrl` returns the redacted form, silently breaking git remotes, reqwest requests, and any transport use | Multi-layered mitigation: (1) explicit `.redacted_string()` / `.raw_string()` methods encourage callers to name the form; (2) clippy `disallowed-methods` entry for `::to_string` pointing callers at the explicit methods; (3) `disallowed-types` ban on `url::Url`/`reqwest::Url` in credential-handling crates forces all URL handling through the typed API; (4) per-call-site test in the sandbox caller (`fabro-sandbox/src/daytona/mod.rs:826`, `fabro-workflow/src/sandbox_git.rs:174`) that asserts the string passed to `shell_quote` contains the raw token, not `****`. | | JSON DTO Serialize leaks raw tokens if serialization matches uv's transparent behavior | Fabro diverges: any serializable redacted-output path renders redacted. Unit 2.1 must decide whether that is a blanket `DisplaySafeUrl` impl or a separate output-only wrapper, and must include a data-loss test/usage audit so persistence paths do not accidentally store `****`. | @@ -948,7 +949,7 @@ impl fabro_options_metadata::OptionsMetadata for RunArgs { - Unit 1.1 + Unit 1.2. Largest mechanical change; lowest semantic risk. - **Why first:** Foundation for hygienic env var handling; unblocks future work (e.g., registry-driven subprocess allowlist in `spawn_env.rs`, doc generation in Phase 6). -### Phase 2: `fabro-util::redact` DisplaySafeUrl (~1 PR) +### Phase 2: `fabro-redact` DisplaySafeUrl (~1 PR) - Unit 2.1 + Unit 2.2. Highest security value. - **Why second:** Independent of Phase 1. Shipping after Phase 1 keeps the mechanical diff separate from the behavior-affecting diff. diff --git a/lib/crates/fabro-auth/Cargo.toml b/lib/crates/fabro-auth/Cargo.toml index 881ef5549..4952bff6f 100644 --- a/lib/crates/fabro-auth/Cargo.toml +++ b/lib/crates/fabro-auth/Cargo.toml @@ -17,8 +17,8 @@ chrono = { workspace = true, features = ["serde"] } fabro-http.workspace = true fabro-model = { path = "../fabro-model" } fabro-oauth = { path = "../fabro-oauth" } +fabro-redact.workspace = true fabro-static.workspace = true -fabro-util = { path = "../fabro-util" } fabro-vault = { path = "../fabro-vault" } serde.workspace = true serde_json.workspace = true diff --git a/lib/crates/fabro-auth/src/credential.rs b/lib/crates/fabro-auth/src/credential.rs index 96df27c88..aed972797 100644 --- a/lib/crates/fabro-auth/src/credential.rs +++ b/lib/crates/fabro-auth/src/credential.rs @@ -1,6 +1,6 @@ use chrono::{DateTime, Duration, Utc}; use fabro_model::Provider; -use fabro_util::redact::redact_string; +use fabro_redact::redact_string; use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] diff --git a/lib/crates/fabro-cli/Cargo.toml b/lib/crates/fabro-cli/Cargo.toml index 1676334d6..51c27fc22 100644 --- a/lib/crates/fabro-cli/Cargo.toml +++ b/lib/crates/fabro-cli/Cargo.toml @@ -45,6 +45,7 @@ fabro-telemetry = { path = "../fabro-telemetry" } fabro-store = { path = "../fabro-store" } fabro-vault = { path = "../fabro-vault" } fabro-types = { path = "../fabro-types", features = ["clap"] } +fabro-redact.workspace = true fabro-util = { path = "../fabro-util" } fabro-http.workspace = true fabro-static.workspace = true diff --git a/lib/crates/fabro-cli/src/commands/run/logs.rs b/lib/crates/fabro-cli/src/commands/run/logs.rs index 11def61ff..e9c4c99ca 100644 --- a/lib/crates/fabro-cli/src/commands/run/logs.rs +++ b/lib/crates/fabro-cli/src/commands/run/logs.rs @@ -13,8 +13,8 @@ use std::time::Duration; use anyhow::{Context, Result, bail}; use chrono::{DateTime, Utc}; +use fabro_redact::redact_jsonl_line; use fabro_util::json::normalize_json_value; -use fabro_util::redact::redact_jsonl_line; use fabro_util::terminal::Styles; use tokio::time; use tracing::{debug, info}; diff --git a/lib/crates/fabro-github/Cargo.toml b/lib/crates/fabro-github/Cargo.toml index 715ab1e01..a3e0a1f0b 100644 --- a/lib/crates/fabro-github/Cargo.toml +++ b/lib/crates/fabro-github/Cargo.toml @@ -16,7 +16,7 @@ workspace = true serde.workspace = true serde_json.workspace = true fabro-http.workspace = true -fabro-util = { path = "../fabro-util" } +fabro-redact.workspace = true fabro-static.workspace = true fabro-types = { path = "../fabro-types" } jsonwebtoken.workspace = true diff --git a/lib/crates/fabro-github/src/lib.rs b/lib/crates/fabro-github/src/lib.rs index 2e5b00860..2c339e7db 100644 --- a/lib/crates/fabro-github/src/lib.rs +++ b/lib/crates/fabro-github/src/lib.rs @@ -1,9 +1,9 @@ use base64::Engine; use base64::engine::general_purpose::STANDARD; +use fabro_redact::DisplaySafeUrl; use fabro_static::EnvVars; use fabro_types::PullRequestGithubDetail; use fabro_types::settings::run::MergeStrategy; -use fabro_util::redact::DisplaySafeUrl; use serde::Deserialize; use tokio::process::Command; diff --git a/lib/crates/fabro-llm/Cargo.toml b/lib/crates/fabro-llm/Cargo.toml index a298c54c4..04258b325 100644 --- a/lib/crates/fabro-llm/Cargo.toml +++ b/lib/crates/fabro-llm/Cargo.toml @@ -35,6 +35,7 @@ tracing.workspace = true fabro-http.workspace = true fabro-auth = { path = "../fabro-auth" } fabro-model = { path = "../fabro-model" } +fabro-redact.workspace = true fabro-static.workspace = true fabro-util = { path = "../fabro-util" } diff --git a/lib/crates/fabro-llm/src/providers/fabro_server.rs b/lib/crates/fabro-llm/src/providers/fabro_server.rs index c76936698..a737ff8f1 100644 --- a/lib/crates/fabro-llm/src/providers/fabro_server.rs +++ b/lib/crates/fabro-llm/src/providers/fabro_server.rs @@ -1,4 +1,4 @@ -use fabro_util::redact::DisplaySafeUrl; +use fabro_redact::DisplaySafeUrl; use futures::stream; use tracing::{debug, error}; diff --git a/lib/crates/fabro-oauth/Cargo.toml b/lib/crates/fabro-oauth/Cargo.toml index 68261dccb..56a8d4f19 100644 --- a/lib/crates/fabro-oauth/Cargo.toml +++ b/lib/crates/fabro-oauth/Cargo.toml @@ -16,6 +16,7 @@ workspace = true serde.workspace = true serde_json.workspace = true fabro-http.workspace = true +fabro-redact.workspace = true sha2.workspace = true base64.workspace = true rand.workspace = true diff --git a/lib/crates/fabro-oauth/src/lib.rs b/lib/crates/fabro-oauth/src/lib.rs index 5de284ceb..e0417007e 100644 --- a/lib/crates/fabro-oauth/src/lib.rs +++ b/lib/crates/fabro-oauth/src/lib.rs @@ -6,8 +6,8 @@ use axum::response::Html; use axum::routing::get; use base64::Engine; use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use fabro_redact::DisplaySafeUrl; use fabro_util::browser; -use fabro_util::redact::DisplaySafeUrl; use serde::Deserialize; use sha2::{Digest, Sha256}; use tokio::net::TcpListener; diff --git a/lib/crates/fabro-redact/Cargo.toml b/lib/crates/fabro-redact/Cargo.toml new file mode 100644 index 000000000..77ec8afe7 --- /dev/null +++ b/lib/crates/fabro-redact/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "fabro-redact" +edition.workspace = true +version.workspace = true +publish = false +license.workspace = true +description = "Secret and credential redaction utilities for Fabro" + +[lib] +doctest = false + +[lints] +workspace = true + +[dependencies] +aho-corasick.workspace = true +ref-cast.workspace = true +regex.workspace = true +serde_json.workspace = true +thiserror.workspace = true +url.workspace = true + +[build-dependencies] +serde = { workspace = true } +toml.workspace = true + +[dev-dependencies] +tracing.workspace = true +tracing-subscriber.workspace = true diff --git a/lib/crates/fabro-util/build.rs b/lib/crates/fabro-redact/build.rs similarity index 100% rename from lib/crates/fabro-util/build.rs rename to lib/crates/fabro-redact/build.rs diff --git a/lib/crates/fabro-util/data/gitleaks.toml b/lib/crates/fabro-redact/data/gitleaks.toml similarity index 100% rename from lib/crates/fabro-util/data/gitleaks.toml rename to lib/crates/fabro-redact/data/gitleaks.toml diff --git a/lib/crates/fabro-util/src/redact/entropy.rs b/lib/crates/fabro-redact/src/entropy.rs similarity index 100% rename from lib/crates/fabro-util/src/redact/entropy.rs rename to lib/crates/fabro-redact/src/entropy.rs diff --git a/lib/crates/fabro-util/src/redact/gitleaks.rs b/lib/crates/fabro-redact/src/gitleaks.rs similarity index 100% rename from lib/crates/fabro-util/src/redact/gitleaks.rs rename to lib/crates/fabro-redact/src/gitleaks.rs diff --git a/lib/crates/fabro-util/src/redact/jsonl.rs b/lib/crates/fabro-redact/src/jsonl.rs similarity index 100% rename from lib/crates/fabro-util/src/redact/jsonl.rs rename to lib/crates/fabro-redact/src/jsonl.rs diff --git a/lib/crates/fabro-util/src/redact/mod.rs b/lib/crates/fabro-redact/src/lib.rs similarity index 93% rename from lib/crates/fabro-util/src/redact/mod.rs rename to lib/crates/fabro-redact/src/lib.rs index 871c003a7..d0fc361c3 100644 --- a/lib/crates/fabro-util/src/redact/mod.rs +++ b/lib/crates/fabro-redact/src/lib.rs @@ -1,3 +1,9 @@ +//! Secret and credential redaction utilities. +//! +//! `redact_string`, `redact_json_value`, and `redact_jsonl_line` provide +//! generic secret scanning. [`DisplaySafeUrl`] provides deterministic URL +//! redaction for logging and error-message boundaries. + mod entropy; mod gitleaks; mod jsonl; diff --git a/lib/crates/fabro-util/src/redact/safe_url.rs b/lib/crates/fabro-redact/src/safe_url.rs similarity index 98% rename from lib/crates/fabro-util/src/redact/safe_url.rs rename to lib/crates/fabro-redact/src/safe_url.rs index cd0637e94..38491ffd7 100644 --- a/lib/crates/fabro-util/src/redact/safe_url.rs +++ b/lib/crates/fabro-redact/src/safe_url.rs @@ -1,6 +1,6 @@ #![allow( clippy::disallowed_types, - reason = "fabro-util::redact owns the raw URL wrapper and redaction boundary" + reason = "fabro-redact owns the raw URL wrapper and redaction boundary" )] //! Credential-redacting URL display helpers. @@ -416,6 +416,13 @@ mod tests { ); } + #[test] + fn display_preserves_ipv6_host_brackets() { + let url = DisplaySafeUrl::parse("https://[::1]:8080/cb?token=abc").unwrap(); + + assert_eq!(url.redacted_string(), "https://[::1]:8080/cb?token=****"); + } + #[test] fn display_redacts_nested_proxy_credentials() { let url = diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml index 51655e3f9..10ebdfafb 100644 --- a/lib/crates/fabro-server/Cargo.toml +++ b/lib/crates/fabro-server/Cargo.toml @@ -36,6 +36,7 @@ fabro-api = { path = "../fabro-api" } fabro-store = { path = "../fabro-store" } fabro-vault = { path = "../fabro-vault" } fabro-http.workspace = true +fabro-redact.workspace = true fabro-static.workspace = true chrono.workspace = true futures-util.workspace = true diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 76f647afa..42b72a700 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -54,6 +54,7 @@ use fabro_llm::types::{ ToolDefinition, }; use fabro_model::{BilledModelUsage, BilledTokenCounts, Catalog}; +use fabro_redact::redact_jsonl_line; use fabro_sandbox::daytona::DaytonaSandbox; use fabro_sandbox::reconnect::reconnect; use fabro_sandbox::{Sandbox, SandboxProvider}; @@ -76,7 +77,6 @@ use fabro_types::{ RunBlobId, RunClientProvenance, RunControlAction, RunEvent, RunId, RunProvenance, RunServerProvenance, RunSubjectProvenance, ServerSettings, }; -use fabro_util::redact::redact_jsonl_line; use fabro_util::text::strip_goal_decoration; use fabro_util::version::FABRO_VERSION; use fabro_vault::{Error as VaultError, SecretType, Vault}; diff --git a/lib/crates/fabro-server/src/web_auth.rs b/lib/crates/fabro-server/src/web_auth.rs index 7b264481a..29439dd8d 100644 --- a/lib/crates/fabro-server/src/web_auth.rs +++ b/lib/crates/fabro-server/src/web_auth.rs @@ -7,11 +7,11 @@ use axum::routing::{get, post}; use axum::{Extension, Json, Router}; use cookie::time::Duration; use cookie::{Cookie, CookieJar, Key, SameSite}; +use fabro_redact::DisplaySafeUrl; use fabro_static::EnvVars; use fabro_types::settings::ServerAuthMethod; use fabro_types::{IdpIdentity, RunAuthMethod}; use fabro_util::dev_token::validate_dev_token_format; -use fabro_util::redact::DisplaySafeUrl; use percent_encoding::{AsciiSet, NON_ALPHANUMERIC, utf8_percent_encode}; use serde::{Deserialize, Serialize}; use serde_json::json; diff --git a/lib/crates/fabro-util/Cargo.toml b/lib/crates/fabro-util/Cargo.toml index 54f904d7d..7c98d9fb0 100644 --- a/lib/crates/fabro-util/Cargo.toml +++ b/lib/crates/fabro-util/Cargo.toml @@ -4,7 +4,7 @@ edition.workspace = true version.workspace = true publish = false license.workspace = true -description = "Shared utilities: secret redaction and terminal styling" +description = "Shared utilities for terminal output, paths, environment, and runtime helpers" [lib] doctest = false @@ -16,11 +16,7 @@ workspace = true console.workspace = true fabro-static.workspace = true rand.workspace = true -ref-cast.workspace = true -regex.workspace = true termimad.workspace = true -thiserror.workspace = true -aho-corasick.workspace = true serde_json.workspace = true serde.workspace = true tokio.workspace = true @@ -29,11 +25,6 @@ tracing.workspace = true tracing-subscriber.workspace = true anyhow.workspace = true open = "5" -url.workspace = true - -[build-dependencies] -toml = "0.8" -serde = { workspace = true } [dev-dependencies] insta = { workspace = true } diff --git a/lib/crates/fabro-util/src/lib.rs b/lib/crates/fabro-util/src/lib.rs index b77536640..8edce1215 100644 --- a/lib/crates/fabro-util/src/lib.rs +++ b/lib/crates/fabro-util/src/lib.rs @@ -8,7 +8,6 @@ pub mod home; pub mod json; pub mod path; pub mod printer; -pub mod redact; pub mod run_log; pub mod session_secret; pub mod terminal; diff --git a/lib/crates/fabro-workflow/Cargo.toml b/lib/crates/fabro-workflow/Cargo.toml index 932e092b4..6dfd414a1 100644 --- a/lib/crates/fabro-workflow/Cargo.toml +++ b/lib/crates/fabro-workflow/Cargo.toml @@ -31,6 +31,7 @@ fabro-github = { path = "../fabro-github" } fabro-interview = { path = "../fabro-interview" } fabro-template = { path = "../fabro-template" } fabro-util = { path = "../fabro-util" } +fabro-redact.workspace = true fabro-checkpoint = { path = "../fabro-checkpoint" } fabro-llm = { path = "../fabro-llm" } fabro-model = { path = "../fabro-model" } diff --git a/lib/crates/fabro-workflow/src/event.rs b/lib/crates/fabro-workflow/src/event.rs index 524c06f7b..e1a55f660 100644 --- a/lib/crates/fabro-workflow/src/event.rs +++ b/lib/crates/fabro-workflow/src/event.rs @@ -13,10 +13,10 @@ use anyhow::{Context, Result}; use chrono::Utc; use fabro_agent::{AgentEvent, SandboxEvent, WorktreeEvent, WorktreeEventCallback}; use fabro_llm::types::TokenCounts as LlmTokenCounts; +use fabro_redact::redact_json_value; use fabro_store::{EventPayload, RunDatabase}; pub use fabro_types::{EventBody, RunNoticeLevel}; use fabro_util::json::normalize_json_value; -use fabro_util::redact::redact_json_value; use serde::{Deserialize, Serialize}; use serde_json::Value; use tokio::io::{AsyncWrite, AsyncWriteExt};