From d5dcd117935b89960973b1ff14cba051f0650c1f Mon Sep 17 00:00:00 2001 From: "fabro-sh-fabro[bot]" <296591931+fabro-sh-fabro[bot]@users.noreply.github.com> Date: Thu, 9 Jul 2026 10:48:02 -0400 Subject: [PATCH] Remove unused provenance tracking from config interpolation (#562) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `Resolved` / `Provenance` types in `fabro-types` interp tracked which env vars and secrets contributed to a resolved value, but no production code ever read `.provenance` — every caller immediately discarded it with `.map(|r| r.value)`. The redaction design this metadata anticipated was superseded by per-run exact-value registration (`fabro_redact::SecretRedactor`); origin-tagging on resolved strings can't reach the surfaces where secrets actually leak (subprocess output, diffs, tool output), so it added no coverage. This PR removes the dead scaffolding with zero behavior change: - `resolve` / `resolve_with` now return `Result` directly; `Resolved` and `Provenance` are deleted along with the name-accumulation logic inside `resolve_with`. - All call sites drop the now-unnecessary `.map(|r| r.value)` unwrap. - Provenance assertions in tests are removed; all value/error assertions are preserved. - The module doc is updated to describe the actual model: secret values are intended to be registered into a per-run exact-value redactor at resolution time; sensitivity is not tracked on resolved strings. - A comment on `ResolvedRunGoal` / `ResolvedGoalSource` (an unrelated run-metadata concept sharing the word "provenance") is rephrased to avoid confusion with the deleted type. `Provenance` no longer appears in `fabro-types/src/settings/mod.rs` exports. The unrelated `RunClientProvenance` / `RunServerProvenance` run-spec types are untouched. ### Fabro Details
Ran 8 stages in 39m 23s for $7.58 | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 1s | – | 0 | | preflight_compile | 2m 17s | – | 0 | | preflight_lint | 2m 33s | – | 0 | | implement | 11m 6s | $4.23 | 0 | | simplify_fable | 8m 27s | $1.60 | 0 | | simplify_gpt | 5m 58s | $1.75 | 0 | | verify | 8m 36s | – | 0 | | **Total** | **39m 23s** | **$7.58** | **0** |
Ran ImplementPlan.fabro (11 nodes and 14 edges) ```dot digraph ImplementPlan { graph [ goal="Implement and simplify", model_stylesheet=" * { model: claude-opus-4-8; } " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD. Be sure to use the rust-style-guide skill to help you follow this repo's Rust style conventions.", model="gpt-55", reasoning_effort="xhigh"] simplify_fable [label="Simplify (Fable)", prompt="@prompts/simplify.md", model="claude-fable-5", reasoning_effort="xhigh"] simplify_gpt [label="Simplify (GPT-55)", prompt="@prompts/simplify.md", model="gpt-55"] verify [label="Verify", shape=parallelogram, timeout="1800s", script="git fetch origin main 2>&1 && git merge --no-edit --no-stat origin/main 2>&1 && cargo +nightly-2026-04-14 fmt --all 2>&1 && cargo dev docs refresh 2>&1 && cargo +nightly-2026-04-14 fmt --check --all 2>&1 && { command -v rg >/dev/null 2>&1 || { echo 'rg is required for verify'; exit 127; }; } && ! rg -n 'AuthMode::Disabled|RunAuthMethod|RunSubjectProvenance|\bActorRef\b|\bActorKind\b|AuthenticatedSubject|AuthenticatedService|AuthorizeRunScoped|AuthorizeRunBlob|AuthorizeStageArtifact|AuthorizeCommandLog|auth_method\s*==\s*\"disabled\"' lib/crates apps lib/packages docs/public/api-reference/fabro-api.yaml 2>&1 && cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings 2>&1 && cargo nextest run --workspace --status-level slow --profile ci 2>&1 && cargo dev docs check 2>&1 && bun install --frozen-lockfile 2>&1 && (cd apps/fabro-web && bun run typecheck) 2>&1 && (cd apps/fabro-web && bun run test) 2>&1 && (cd lib/packages/fabro-api-client && bun run typecheck) 2>&1 && cargo dev build -- -p fabro-cli --release 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all format, clippy, Rust test, docs, TypeScript typecheck/test, and build failures.", max_visits=3] start -> toolchain toolchain -> preflight_compile [condition="outcome=succeeded"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=succeeded"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=succeeded"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_fable -> simplify_gpt -> verify verify -> exit [condition="outcome=succeeded"] verify -> fixup fixup -> verify } ```
⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro --- lib/crates/fabro-config/src/run.rs | 2 +- lib/crates/fabro-hooks/src/executor.rs | 4 +- lib/crates/fabro-server/src/server.rs | 9 +- lib/crates/fabro-types/src/settings/interp.rs | 94 ++++--------------- lib/crates/fabro-types/src/settings/mod.rs | 2 +- lib/crates/fabro-types/src/settings/run.rs | 13 +-- 6 files changed, 28 insertions(+), 96 deletions(-) diff --git a/lib/crates/fabro-config/src/run.rs b/lib/crates/fabro-config/src/run.rs index 691200293..d6114b835 100644 --- a/lib/crates/fabro-config/src/run.rs +++ b/lib/crates/fabro-config/src/run.rs @@ -121,7 +121,7 @@ fn resolve_goal_file( let resolved = file .resolve(process_env_var) .map_err(|err| ResolveRunGoalError::EnvLookup { var: err.name })?; - let path = resolve_goal_file_path(&resolved.value, base_dir); + let path = resolve_goal_file_path(&resolved, base_dir); let text = std::fs::read_to_string(&path).map_err(|source| ResolveRunGoalError::Io { path: path.clone(), source, diff --git a/lib/crates/fabro-hooks/src/executor.rs b/lib/crates/fabro-hooks/src/executor.rs index 9fda710d5..a409a49b7 100644 --- a/lib/crates/fabro-hooks/src/executor.rs +++ b/lib/crates/fabro-hooks/src/executor.rs @@ -77,9 +77,7 @@ fn resolve_interp(value: &InterpString, env: &E) -> Result resolved.value, + Ok(resolved) => resolved, Err(err) => { warn!( run_id = %run_id, @@ -2425,12 +2425,7 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result) -> Result { + pub fn resolve_with(&self, ctx: &mut ResolveCtx<'_>) -> Result { let mut value = String::new(); - let mut env_names = Vec::new(); - let mut secret_names = Vec::new(); for seg in &self.segments { match seg { Segment::Literal(text) => value.push_str(text), @@ -298,19 +297,11 @@ impl InterpString { return Err(ResolveError::missing(*namespace, name)); }; value.push_str(&resolved); - match namespace { - Namespace::Env => env_names.push(name.clone()), - Namespace::Secrets => secret_names.push(name.clone()), - Namespace::Vars | Namespace::Inputs => {} - } } } } - Ok(Resolved { - value, - provenance: Provenance::from_names(env_names, secret_names), - }) + Ok(value) } /// Substitute tokens for the namespaces `ctx` provides, preserving tokens @@ -347,7 +338,7 @@ impl InterpString { /// `lookup` should return the current value for a given env var name (or /// `None` if unset). Tokens in any other namespace fail with /// [`ResolveErrorKind::Unavailable`]. - pub fn resolve(&self, lookup: F) -> Result + pub fn resolve(&self, lookup: F) -> Result where F: FnMut(&str) -> Option, { @@ -368,8 +359,7 @@ impl InterpString { where F: FnMut(&str) -> Option, { - self.resolve(lookup) - .map_or_else(|_| self.as_source(), |resolved| resolved.value) + self.resolve(lookup).unwrap_or_else(|_| self.as_source()) } /// Substitute only `{{ vars.* }}` tokens while preserving all other @@ -426,40 +416,6 @@ impl From<&str> for InterpString { } } -/// The outcome of a successful interpolation resolution. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Resolved { - pub value: String, - pub provenance: Provenance, -} - -/// Provenance metadata for resolved config values. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Provenance { - /// No env var or secret contributed to this value. - Literal, - /// One or more env vars and/or secrets contributed to this value. Used by - /// outward-facing renderers to redact sensitive-sourced values uniformly. - /// `vars`/`inputs` are non-sensitive and do not mark a value as sourced. - Sourced { - env_names: Vec, - secret_names: Vec, - }, -} - -impl Provenance { - fn from_names(env_names: Vec, secret_names: Vec) -> Self { - if env_names.is_empty() && secret_names.is_empty() { - Self::Literal - } else { - Self::Sourced { - env_names, - secret_names, - } - } - } -} - /// An error from resolving or substituting interpolation tokens. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ResolveError { @@ -614,8 +570,7 @@ mod tests { fn resolve_literal_string() { let s = InterpString::parse("static"); let resolved = s.resolve(lookup_from(&[])).unwrap(); - assert_eq!(resolved.value, "static"); - assert_eq!(resolved.provenance, Provenance::Literal); + assert_eq!(resolved, "static"); } #[test] @@ -624,18 +579,14 @@ mod tests { let resolved = s .resolve(lookup_from(&[("API_KEY", "secret-123")])) .unwrap(); - assert_eq!(resolved.value, "secret-123"); - assert_eq!(resolved.provenance, Provenance::Sourced { - env_names: vec!["API_KEY".into()], - secret_names: vec![], - }); + assert_eq!(resolved, "secret-123"); } #[test] fn resolve_substring() { let s = InterpString::parse("Bearer {{ env.TOKEN }}"); let resolved = s.resolve(lookup_from(&[("TOKEN", "abc")])).unwrap(); - assert_eq!(resolved.value, "Bearer abc"); + assert_eq!(resolved, "Bearer abc"); } #[test] @@ -644,11 +595,7 @@ mod tests { let resolved = s .resolve(lookup_from(&[("USER", "root"), ("HOST", "example.com")])) .unwrap(); - assert_eq!(resolved.value, "root@example.com"); - assert_eq!(resolved.provenance, Provenance::Sourced { - env_names: vec!["USER".into(), "HOST".into()], - secret_names: vec![], - }); + assert_eq!(resolved, "root@example.com"); } #[test] @@ -668,8 +615,7 @@ mod tests { fn unterminated_token_treated_as_literal() { let s = InterpString::parse("{{ env.OPEN"); let resolved = s.resolve(lookup_from(&[])).unwrap(); - assert_eq!(resolved.value, "{{ env.OPEN"); - assert_eq!(resolved.provenance, Provenance::Literal); + assert_eq!(resolved, "{{ env.OPEN"); } #[test] @@ -685,7 +631,7 @@ mod tests { let s = InterpString::parse(raw); assert!(s.is_literal(), "{raw} should stay literal"); let resolved = s.resolve(lookup_from(&[])).unwrap(); - assert_eq!(resolved.value, raw); + assert_eq!(resolved, raw); } } @@ -736,11 +682,7 @@ mod tests { ) .unwrap(); - assert_eq!(resolved.value, "https://us-east-1.example.com"); - assert_eq!(resolved.provenance, Provenance::Sourced { - env_names: vec!["REGION".into()], - secret_names: vec![], - }); + assert_eq!(resolved, "https://us-east-1.example.com"); } #[test] @@ -796,7 +738,7 @@ mod tests { } #[test] - fn resolve_with_secrets_tracks_provenance() { + fn resolve_with_substitutes_secrets_and_env() { let s = InterpString::parse("Bearer {{ secrets.API_KEY }} via {{ env.PROXY }}"); let resolved = s @@ -807,11 +749,7 @@ mod tests { ) .unwrap(); - assert_eq!(resolved.value, "Bearer vault-value via proxy.internal"); - assert_eq!(resolved.provenance, Provenance::Sourced { - env_names: vec!["PROXY".into()], - secret_names: vec!["API_KEY".into()], - }); + assert_eq!(resolved, "Bearer vault-value via proxy.internal"); } #[test] diff --git a/lib/crates/fabro-types/src/settings/mod.rs b/lib/crates/fabro-types/src/settings/mod.rs index 872fa3519..5bf91e4d9 100644 --- a/lib/crates/fabro-types/src/settings/mod.rs +++ b/lib/crates/fabro-types/src/settings/mod.rs @@ -25,7 +25,7 @@ pub use cli::{ CliLoggingSettings, CliNamespace, CliOutputSettings, CliTargetSettings, CliUpdatesSettings, }; pub use duration::{Duration, ParseDurationError}; -pub use interp::{InterpString, Provenance, ResolveCtx, ResolveError, ResolveErrorKind, Resolved}; +pub use interp::{InterpString, ResolveCtx, ResolveError, ResolveErrorKind}; pub use model_ref::{ AmbiguousModelRef, ModelRef, ModelRegistry, ParseModelRefError, ResolvedModelRef, }; diff --git a/lib/crates/fabro-types/src/settings/run.rs b/lib/crates/fabro-types/src/settings/run.rs index 6befaf457..51410c44d 100644 --- a/lib/crates/fabro-types/src/settings/run.rs +++ b/lib/crates/fabro-types/src/settings/run.rs @@ -1110,7 +1110,7 @@ impl RunEnvironmentSettings { for (name, value) in &self.env { let references_secrets = value.references(Namespace::Secrets); let resolved_value = match value.resolve_with(&mut ctx) { - Ok(resolved) => resolved.value, + Ok(resolved) => resolved, Err(err) if err.namespace == Namespace::Env && !references_secrets => { #[expect( clippy::disallowed_methods, @@ -1654,7 +1654,7 @@ fn resolve_env_string( let mut ctx = ResolveCtx::new() .with_env(&mut *env_lookup) .with_secrets(&mut *secrets_lookup); - *value = InterpString::parse(value).resolve_with(&mut ctx)?.value; + *value = InterpString::parse(value).resolve_with(&mut ctx)?; Ok(()) } @@ -2347,21 +2347,22 @@ pub struct ArtifactsSettings { } /// Outcome of resolving a [`RunGoal`] to its final goal text. /// -/// Carries provenance alongside the text so downstream consumers (e.g. the -/// run manifest builder) can distinguish inline goals from file-sourced goals. +/// Carries source metadata alongside the text so downstream consumers (e.g. +/// the run manifest builder) can distinguish inline goals from file-sourced +/// goals. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ResolvedRunGoal { pub text: String, pub source: ResolvedGoalSource, } -/// Provenance of a [`ResolvedRunGoal`]. +/// Source metadata for a [`ResolvedRunGoal`]. #[derive(Debug, Clone, PartialEq, Eq)] pub enum ResolvedGoalSource { /// Goal text came from a literal `run.goal = "..."` value. Inline, /// Goal text was read from a file on disk. The absolute path of that - /// file is carried for provenance / error reporting. + /// file is carried for error reporting. File { path: std::path::PathBuf }, }