mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
Remove unused SecretRedactor registry and update stale comments (#574)
The exact-value secret registry (`SecretRedactor`) was built as
infrastructure ahead of its wiring, but the wiring was never merged —
the team settled on content-based redaction (entropy analysis + gitleaks
patterns) as the sole mechanism. The type had zero consumers outside its
own crate. This PR removes it and corrects two doc comments that
described the abandoned design as pending.
**What changed:**
1. `fabro-redact/src/secret_registry.rs` deleted in full (~217 lines),
with its `mod` declaration and `pub use` re-export removed from
`lib.rs`. `Region`, `redact_string`, `redact_json_value`,
`DisplaySafeUrl`, and everything else in the crate are untouched.
2. The `resolve_extra_headers` doc in `fabro-auth` no longer promises
future exact-match registration. It now honestly states that low-entropy
header values not shaped like credentials are not caught by
content-based redaction.
3. The `InterpString` module doc in `fabro-types` no longer describes a
pending per-run registry. It states the real architecture: resolved
secret values are plain strings, and redaction is content-based applied
at output serialization.
**Known limitation (pre-existing, not introduced here):** a declared
secret whose value is a low-entropy ordinary word (e.g. an environment
name) is not caught by content-based detection. This was the gap
`SecretRedactor` was meant to fill; it is an accepted trade-off, not a
regression from this PR.
### Fabro Details
<details>
<summary>Ran 8 stages in 26m 21s for $2.27</summary>
| Stage | Duration | Cost | Retries |
|---|---|---|---|
| start | 0s | – | 0 |
| toolchain | 1s | – | 0 |
| preflight_compile | 2m 34s | – | 0 |
| preflight_lint | 2m 45s | – | 0 |
| implement | 0s | – | 0 |
| simplify_fable | 9m 44s | $2.27 | 0 |
| simplify_gpt | 0s | – | 0 |
| verify | 10m 50s | – | 0 |
| **Total** | **26m 21s** | **$2.27** | **0** |
</details>
<details>
<summary>Ran <code>ImplementPlan.fabro</code> (11 nodes and 14
edges)</summary>
```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
}
```
</details>
⚒️ Generated with [Fabro](https://fabro.sh)
---------
Co-authored-by: Fabro <noreply@fabro.sh>
This commit is contained in:
fabro-sh-fabro[bot]
GitHub
parent
52d8c01c2a
commit
eccbed80b7
4 changed files with 8 additions and 225 deletions
|
|
@ -477,9 +477,10 @@ impl CredentialResolver {
|
|||
|
||||
/// Resolve a provider's `extra_headers` interpolation sources with `ctx`.
|
||||
///
|
||||
/// Provider header secrets resolve outside the run-boundary redactor
|
||||
/// registration path. Keep this path free of value logging until exact-match
|
||||
/// registration is threaded through.
|
||||
/// Resolved header values may contain secrets; keep this path free of value
|
||||
/// logging. Content-based redaction covers credential-shaped values on output
|
||||
/// surfaces, but no mechanism redacts these exact values, so a low-entropy
|
||||
/// header value that does not look like a credential is not caught.
|
||||
pub(crate) fn resolve_extra_headers(
|
||||
provider: &ProviderId,
|
||||
headers: &HashMap<String, String>,
|
||||
|
|
|
|||
|
|
@ -8,11 +8,9 @@ mod entropy;
|
|||
mod gitleaks;
|
||||
mod jsonl;
|
||||
mod safe_url;
|
||||
mod secret_registry;
|
||||
|
||||
pub use jsonl::{redact_json_value, redact_jsonl_line};
|
||||
pub use safe_url::{DisplaySafeUrl, DisplaySafeUrlError};
|
||||
pub use secret_registry::SecretRedactor;
|
||||
|
||||
pub(crate) const REDACTION_MARKER: &str = "REDACTED";
|
||||
|
||||
|
|
|
|||
|
|
@ -1,217 +0,0 @@
|
|||
use std::sync::{Arc, PoisonError, RwLock, RwLockReadGuard, RwLockWriteGuard};
|
||||
|
||||
use serde_json::Value;
|
||||
|
||||
use crate::Region;
|
||||
|
||||
/// Per-run registry of exact secret values to redact from strings and JSON.
|
||||
///
|
||||
/// This complements the crate's content-based redaction by redacting registered
|
||||
/// values even when they do not look like credentials. Clones share the same
|
||||
/// registry so callers can hand a redactor to another subsystem and continue to
|
||||
/// register values through the original. Registered values are exact substring
|
||||
/// matches and may be low-entropy strings such as environment names.
|
||||
#[derive(Clone, Default)]
|
||||
pub struct SecretRedactor {
|
||||
values: Arc<RwLock<Vec<String>>>,
|
||||
}
|
||||
|
||||
impl SecretRedactor {
|
||||
/// Register a secret value for exact substring redaction.
|
||||
///
|
||||
/// Empty or whitespace-only values are ignored so an accidental empty
|
||||
/// registration cannot redact every output boundary.
|
||||
pub fn register(&self, value: impl Into<String>) {
|
||||
let value = value.into();
|
||||
if value.trim().is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
let mut values = self.write();
|
||||
if !values.contains(&value) {
|
||||
values.push(value);
|
||||
}
|
||||
}
|
||||
|
||||
/// Return `true` when no secret values have been registered.
|
||||
pub fn is_empty(&self) -> bool {
|
||||
self.read().is_empty()
|
||||
}
|
||||
|
||||
/// Redact all registered secret values from `s`.
|
||||
pub fn redact_into(&self, s: &str) -> String {
|
||||
let Some(values) = self.values_snapshot() else {
|
||||
return s.to_string();
|
||||
};
|
||||
redact_string_values(s, &values)
|
||||
}
|
||||
|
||||
/// Redact registered secret values from every JSON string value.
|
||||
///
|
||||
/// Object keys and non-string values are left unchanged.
|
||||
pub fn redact_json(&self, mut value: Value) -> Value {
|
||||
let Some(values) = self.values_snapshot() else {
|
||||
return value;
|
||||
};
|
||||
|
||||
redact_json_leaves(&mut value, &values);
|
||||
value
|
||||
}
|
||||
|
||||
fn read(&self) -> RwLockReadGuard<'_, Vec<String>> {
|
||||
self.values.read().unwrap_or_else(PoisonError::into_inner)
|
||||
}
|
||||
|
||||
fn write(&self) -> RwLockWriteGuard<'_, Vec<String>> {
|
||||
self.values.write().unwrap_or_else(PoisonError::into_inner)
|
||||
}
|
||||
|
||||
fn values_snapshot(&self) -> Option<Vec<String>> {
|
||||
let values = self.read();
|
||||
if values.is_empty() {
|
||||
return None;
|
||||
}
|
||||
Some(values.clone())
|
||||
}
|
||||
}
|
||||
|
||||
fn redact_json_leaves(value: &mut Value, values: &[String]) {
|
||||
match value {
|
||||
Value::Object(obj) => {
|
||||
for child in obj.values_mut() {
|
||||
redact_json_leaves(child, values);
|
||||
}
|
||||
}
|
||||
Value::Array(arr) => {
|
||||
for child in arr {
|
||||
redact_json_leaves(child, values);
|
||||
}
|
||||
}
|
||||
Value::String(text) => {
|
||||
let redacted = redact_string_values(text, values);
|
||||
if redacted != *text {
|
||||
*text = redacted;
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
/// Collect every match of each registered value and let
|
||||
/// [`crate::redact_regions`] sort and merge overlaps, so a secret that overlaps
|
||||
/// another is fully redacted.
|
||||
///
|
||||
/// Assumes a small number of registered values (bounded by the run's declared
|
||||
/// secrets), so the per-value scan is not optimized further.
|
||||
fn redact_string_values(s: &str, values: &[String]) -> String {
|
||||
let mut regions = Vec::new();
|
||||
for value in values {
|
||||
for (start, _) in s.match_indices(value) {
|
||||
regions.push(Region {
|
||||
start,
|
||||
end: start + value.len(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if regions.is_empty() {
|
||||
return s.to_string();
|
||||
}
|
||||
|
||||
crate::redact_regions(s, regions)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use serde_json::json;
|
||||
|
||||
use super::SecretRedactor;
|
||||
|
||||
#[test]
|
||||
fn redacts_registered_low_entropy_value() {
|
||||
let redactor = SecretRedactor::default();
|
||||
redactor.register("staging");
|
||||
|
||||
assert_eq!(
|
||||
crate::redact_string("deploy to staging"),
|
||||
"deploy to staging"
|
||||
);
|
||||
assert_eq!(
|
||||
redactor.redact_into("deploy to staging"),
|
||||
"deploy to REDACTED"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignores_empty_and_whitespace_values() {
|
||||
let redactor = SecretRedactor::default();
|
||||
redactor.register("");
|
||||
redactor.register(" ");
|
||||
|
||||
assert_eq!(
|
||||
redactor.redact_into("deploy to staging"),
|
||||
"deploy to staging"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redacts_overlapping_values_longest_first() {
|
||||
let redactor = SecretRedactor::default();
|
||||
redactor.register("abc");
|
||||
redactor.register("abcdef");
|
||||
|
||||
assert_eq!(redactor.redact_into("token=abcdef"), "token=REDACTED");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_registry_is_identity() {
|
||||
let redactor = SecretRedactor::default();
|
||||
let value = json!({
|
||||
"env": "staging",
|
||||
"items": ["staging", 42],
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
redactor.redact_into("deploy to staging"),
|
||||
"deploy to staging"
|
||||
);
|
||||
assert_eq!(redactor.redact_json(value.clone()), value);
|
||||
assert!(redactor.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redact_json_redacts_nested_object_values_and_array_elements() {
|
||||
let redactor = SecretRedactor::default();
|
||||
redactor.register("staging");
|
||||
let value = json!({
|
||||
"environment": "staging",
|
||||
"items": [
|
||||
"keep",
|
||||
"deploy staging now"
|
||||
],
|
||||
"staging": "object keys are not redacted",
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
redactor.redact_json(value),
|
||||
json!({
|
||||
"environment": "REDACTED",
|
||||
"items": [
|
||||
"keep",
|
||||
"deploy REDACTED now"
|
||||
],
|
||||
"staging": "object keys are not redacted",
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clones_share_registered_values() {
|
||||
let redactor = SecretRedactor::default();
|
||||
let clone = redactor.clone();
|
||||
|
||||
redactor.register("staging");
|
||||
|
||||
assert_eq!(clone.redact_into("deploy to staging"), "deploy to REDACTED");
|
||||
}
|
||||
}
|
||||
|
|
@ -14,9 +14,10 @@
|
|||
//! Resolution timing is split: `vars` substitutes early (server-side, at run
|
||||
//! creation) via [`InterpString::substitute_with`], while `env`/`secrets`
|
||||
//! resolve late, at consumption time in the process that owns
|
||||
//! the value, via [`InterpString::resolve_with`]. Declared-secret values are
|
||||
//! intended to be registered into a per-run exact-value redactor where secrets
|
||||
//! resolve; sensitivity is not tracked on resolved strings.
|
||||
//! the value, via [`InterpString::resolve_with`]. Resolved secret values are
|
||||
//! plain strings; sensitivity is not tracked. Redaction of run output is
|
||||
//! content-based (entropy analysis plus credential patterns), applied where
|
||||
//! output is serialized.
|
||||
|
||||
use std::borrow::Cow;
|
||||
use std::fmt;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue