Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions crates/allow-core/src/finding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,21 @@ impl StructuralIdentity {
cap_opt(&mut self.target_fingerprint);
}

/// Redact the source-text-bearing identity fields (`symbol`, `callee`,
/// `container`, `module`, `macro_name`, `lint`) by clearing them, while
/// preserving the structural anchors (`normalized_snippet_hash`,
/// fingerprints, `ast_kind`, `line_hint`, `column_hint`) that matching
/// relies on. Opt-in for CI artifacts where source-text-derived fields are
/// an info-leak surface (#1920).
pub fn redact_source_text_fields(&mut self) {
self.symbol = None;
self.callee = None;
self.container = None;
self.module = None;
self.macro_name = None;
self.lint = None;
}

Comment on lines +160 to +174

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -B2 -A6 '\.stable_key\(\)' --type=rust
rg -n -B2 -A10 'fn stable_key_parts' --type=rust
rg -n -B2 -A10 'stable_identity_key_from_parts' --type=rust

Repository: EffortlessMetrics/cargo-allow

Length of output: 167


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== finding.rs outline ==\n'
ast-grep outline crates/allow-core/src/finding.rs --view expanded || true

printf '\n== finding.rs lines around redaction ==\n'
sed -n '1,260p' crates/allow-core/src/finding.rs | cat -n | sed -n '120,230p'

printf '\n== repository search for stable_key symbols ==\n'
rg -n --hidden --glob '!target' 'stable_key|stable_identity_key|identity key|redact_source_text_fields|CARGO_ALLOW_REDACT_IDENTITY|unwrap_or_default\(\)' crates . || true

printf '\n== repository search for allow-list matching ==\n'
rg -n --hidden --glob '!target' 'allow[-_ ]list|allowlist|matching|suppress|suppression|permit' crates . || true

Repository: EffortlessMetrics/cargo-allow

Length of output: 50386


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== finding.rs relevant functions =='
sed -n '224,260p' crates/allow-core/src/finding.rs | cat -n

echo
echo '== exact stable_key/finding_identity_key call sites in core =='
rg -n -B2 -A4 'finding_identity_key\(|stable_key\(' crates/allow-core crates/allow-policy crates/allow-report crates/cargo-allow crates/allow-rust --type=rust

echo
echo '== tests and docs mentioning hints / redaction =='
rg -n -B2 -A4 'line_hint|column_hint|redact_source_text_fields|CARGO_ALLOW_REDACT_IDENTITY|stable identity key|finding identity key' crates/allow-core crates/allow-report docs --type=rust --type=md

echo
echo '== specific use of stable_key in allow-rust tests or production =='
rg -n -B2 -A6 'identity\.stable_key\(\)|finding_identity_key\(' crates/allow-rust crates/cargo-allow crates/allow-policy --type=rust

Repository: EffortlessMetrics/cargo-allow

Length of output: 50387


Redaction should preserve matching entropy

redact_source_text_fields() clears fields that still feed stable_key()/finding_identity_key() (module, container, symbol, callee, macro_name, lint). line_hint/column_hint are review hints only, not part of the key. If redacted findings participate in matching, distinct findings on the same path can collapse onto the same key; replace the cleared text with a stable digest or keep redaction out of matching inputs.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/allow-core/src/finding.rs` around lines 160 - 174, The redaction in
redact_source_text_fields() is removing fields that still participate in
stable_key() and finding_identity_key(), which can make distinct findings
collide after redaction. Update Finding so redacted text fields like symbol,
callee, container, module, macro_name, and lint are either replaced with a
stable digest/value that preserves uniqueness or are excluded from the matching
key computation, while keeping the structural anchors used for matching intact.

pub fn stable_key(&self) -> String {
stable_identity_key_from_parts(self.stable_key_parts())
}
Expand Down
39 changes: 39 additions & 0 deletions crates/allow-core/src/finding_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,42 @@ fn truncate_in_place_leaves_short_fields_unchanged() {
assert_eq!(identity.ast_kind, "method_call");
assert_eq!(identity.symbol.as_deref(), Some("unwrap"));
}

#[test]
fn redact_source_text_fields_clears_text_but_preserves_anchors() {
// #1920: redaction clears the source-text-bearing fields (info-leak
// surface) while preserving the structural anchors matching relies on.
let mut identity = StructuralIdentity::new("rust", "method_call");
identity.symbol = Some("secret_token".to_string());
identity.callee = Some("leaky_call".to_string());
identity.container = Some("my_impl".to_string());
identity.module = Some("my_mod".to_string());
identity.macro_name = Some("my_macro".to_string());
identity.lint = Some("clippy::foo".to_string());
identity.normalized_snippet_hash = Some("fnv1a64:abc".to_string());
identity.receiver_fingerprint = Some("rx".to_string());
identity.target_fingerprint = Some("tx".to_string());
identity.line_hint = Some(42);
identity.column_hint = Some(7);

identity.redact_source_text_fields();

// Source-text-bearing fields cleared.
assert_eq!(identity.symbol, None);
assert_eq!(identity.callee, None);
assert_eq!(identity.container, None);
assert_eq!(identity.module, None);
assert_eq!(identity.macro_name, None);
assert_eq!(identity.lint, None);
// Structural anchors preserved (matching still works).
assert_eq!(identity.language, "rust");
assert_eq!(identity.ast_kind, "method_call");
assert_eq!(
identity.normalized_snippet_hash.as_deref(),
Some("fnv1a64:abc")
);
assert_eq!(identity.receiver_fingerprint.as_deref(), Some("rx"));
assert_eq!(identity.target_fingerprint.as_deref(), Some("tx"));
assert_eq!(identity.line_hint, Some(42));
assert_eq!(identity.column_hint, Some(7));
}
3 changes: 2 additions & 1 deletion crates/allow-report/src/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ pub const CLAIM_BOUNDARY: &[&str] = &[
"data_flow_not_analyzed",
"external_evidence_tools_not_invoked",
"repository_code_not_executed",
"source_text_in_identity_fields",
];

pub const SCANNER_LIMITATIONS: &[&str] = &[
Expand Down Expand Up @@ -287,7 +288,7 @@ pub fn scanner_limitations_for_schema_id(schema_id: &str) -> &'static [&'static
}
}

pub const CLAIM_BOUNDARY_TEXT: &str = "Claim boundary: scanned source-tree/source syntax only; cargo-allow did not invoke Cargo metadata, Cargo commands, rustc, Clippy, build scripts, proc macros, external evidence tools, or repository code. Macro expansion, macro token-tree contents, type information, MIR, build output, control flow, and data flow were not analyzed.";
pub const CLAIM_BOUNDARY_TEXT: &str = "Claim boundary: scanned source-tree/source syntax only; cargo-allow did not invoke Cargo metadata, Cargo commands, rustc, Clippy, build scripts, proc macros, external evidence tools, or repository code. Macro expansion, macro token-tree contents, type information, MIR, build output, control flow, and data flow were not analyzed. Identity fields (symbol, callee, container, module, macro_name, lint) carry source-derived text and are emitted in CI artifacts; set CARGO_ALLOW_REDACT_IDENTITY=1 to redact them (structural hashes are preserved for matching).";

#[derive(Debug, Clone, Copy)]
pub struct InventoryContext<'a> {
Expand Down
2 changes: 1 addition & 1 deletion crates/allow-report/src/json_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fn json_contains_claim_boundary() {
);
assert!(CLAIM_BOUNDARY.contains(&"source_tree_inventory"));
assert!(SCANNER_LIMITATIONS.contains(&"cargo_metadata_not_invoked"));
assert_eq!(CLAIM_BOUNDARY.len(), SCANNER_LIMITATIONS.len() + 2);
assert_eq!(CLAIM_BOUNDARY.len(), SCANNER_LIMITATIONS.len() + 3);
assert!(json.contains("source_tree_inventory"));
assert!(json.contains("cargo_metadata_not_invoked"));
assert!(json.contains("cargo_commands_not_invoked"));
Expand Down
6 changes: 6 additions & 0 deletions crates/allow-rust/src/finding_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ pub(crate) fn push_finding<F>(
// Cap source-derived string fields so a megabyte-long identifier cannot
// inflate report/receipt artifacts unboundedly (#1919).
identity.truncate_in_place();
// Opt-in: redact source-text-bearing identity fields for CI artifacts
// where they are an info-leak surface. Structural hashes are preserved.
// (#1920)
if std::env::var("CARGO_ALLOW_REDACT_IDENTITY").as_deref() == Ok("1") {
identity.redact_source_text_fields();
Comment on lines +43 to +44

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid redacting identities before policy matching

When CARGO_ALLOW_REDACT_IDENTITY=1 is set for the intended CI artifact use case, this clears callee, container, symbol, lint, etc. while the findings are still the inputs to allow_match::score_match, whose selector checks hard-gate on those exact fields. Existing allow entries generated from normal findings commonly include those selectors, so a previously receipted unwrap/unsafe/lint finding will become New and the entry Stale under the redaction env var even though the code did not change. Redaction needs to happen only at artifact serialization/output time, or matching must use an unredacted copy.

Useful? React with 👍 / 👎.

}
Comment on lines +40 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== repo files of interest ==\n'
git ls-files | rg '(^|/)(rust-toolchain(\.toml)?|Cargo\.toml|Cargo\.lock|finding_builder\.rs)$'

printf '\n== rust toolchain / msrv hints ==\n'
for f in rust-toolchain.toml rust-toolchain Cargo.toml crates/allow-rust/Cargo.toml; do
  if [ -f "$f" ]; then
    echo "--- $f"
    cat -n "$f" | sed -n '1,220p'
  fi
done

printf '\n== finding_builder.rs ==\n'
cat -n crates/allow-rust/src/finding_builder.rs | sed -n '1,220p'

Repository: EffortlessMetrics/cargo-allow

Length of output: 5735


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== push_finding usages ==\n'
rg -n "push_finding\(" crates/allow-rust -g '!target' -g '!**/node_modules/**'

printf '\n== structural identity / redact flag related symbols ==\n'
rg -n "CARGO_ALLOW_REDACT_IDENTITY|redact_source_text_fields|OnceLock|push_finding" crates/allow-rust -g '!target' -g '!**/node_modules/**'

printf '\n== relevant surrounding files ==\n'
for f in crates/allow-rust/src/*.rs; do
  echo "--- $f"
  sed -n '1,220p' "$f" | rg -n "push_finding|CARGO_ALLOW_REDACT_IDENTITY|redact_source_text_fields|OnceLock|StructuralIdentity|FindingSite|FindingKind"
done

Repository: EffortlessMetrics/cargo-allow

Length of output: 3188


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import os, re, pathlib, json
root = pathlib.Path('.')

candidates = [
    'rust-toolchain.toml',
    'rust-toolchain',
    'Cargo.toml',
    'crates/allow-rust/Cargo.toml',
]
for p in candidates:
    path = root / p
    if path.exists():
        print(f'--- {p}')
        text = path.read_text(errors='replace').splitlines()
        for i, line in enumerate(text[:220], 1):
            if re.search(r'(toolchain|rust-version|edition|msrv|minimum supported rust)', line, re.I):
                print(f'{i}:{line}')
        # print top relevant context for rust-toolchain/Cargo
        if p.endswith('rust-toolchain.toml') or p == 'rust-toolchain' or p.endswith('Cargo.toml'):
            for i, line in enumerate(text[:80], 1):
                print(f'{i}:{line}')
                if i >= 80:
                    break
PY

printf '\n== finding_builder relevant lines ==\n'
sed -n '1,120p' crates/allow-rust/src/finding_builder.rs | nl -ba | sed -n '1,120p'

printf '\n== callers/usages ==\n'
rg -n "push_finding\(" crates/allow-rust -g '!target' -g '!**/node_modules/**'

printf '\n== env/redact symbols ==\n'
rg -n "CARGO_ALLOW_REDACT_IDENTITY|redact_source_text_fields|OnceLock" crates/allow-rust -g '!target' -g '!**/node_modules/**'

Repository: EffortlessMetrics/cargo-allow

Length of output: 2509


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== toolchain manifests =="
for f in rust-toolchain.toml rust-toolchain Cargo.toml crates/allow-rust/Cargo.toml; do
  if [ -f "$f" ]; then
    echo "--- $f"
    nl -ba "$f" | sed -n '1,160p'
  fi
done

echo
echo "== finding_builder.rs excerpt =="
nl -ba crates/allow-rust/src/finding_builder.rs | sed -n '1,140p'

echo
echo "== push_finding call sites =="
rg -n "push_finding\(" crates/allow-rust

echo
echo "== OnceLock usage =="
rg -n "OnceLock|CARGO_ALLOW_REDACT_IDENTITY|redact_source_text_fields" crates/allow-rust

Repository: EffortlessMetrics/cargo-allow

Length of output: 248


🌐 Web query:

Rust std::sync::OnceLock stabilized version

💡 Result:

The Rust standard library structure std::sync::OnceLock was stabilized in version 1.70.0 [1][2][3]. It is a thread-safe synchronization primitive that can be written to only once, functioning as a thread-safe alternative to OnceCell [1][4]. The stabilization was implemented through pull request #105587 [5]. At the time of its initial stabilization, other related types like LazyLock and LazyCell were not included and remained experimental, later receiving their own stabilization in subsequent releases [5].

Citations:


Cache the redaction flag once. push_finding reads CARGO_ALLOW_REDACT_IDENTITY for every finding, which is avoidable work in the scanner hot path. allow-rust targets Rust 1.85, so std::sync::OnceLock<bool> is available here.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/allow-rust/src/finding_builder.rs` around lines 40 - 45, Cache the
redaction flag once instead of reading CARGO_ALLOW_REDACT_IDENTITY on every
push_finding call, since this is on the scanner hot path. Add a OnceLock<bool>
helper near finding_builder::push_finding or the redaction branch to initialize
the env check once, then reuse that cached value before calling
identity.redact_source_text_fields(). Keep the behavior the same, just move the
env lookup out of the per-finding path.

findings.push(Finding {
kind,
family: Some(family.to_string()),
Expand Down
1 change: 1 addition & 0 deletions docs/schemas/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ artifact's scan surface, not limitations.
| `data_flow_not_analyzed` | Yes | data-flow analysis was not performed. |
| `external_evidence_tools_not_invoked` | Yes | External evidence tools can be referenced by policy but were not executed by the scan. |
| `repository_code_not_executed` | Yes | Repository code was not executed by cargo-allow. |
| `source_text_in_identity_fields` | No | Identity fields (symbol, callee, container, module, macro_name, lint) carry source-derived text and are emitted in CI artifacts; set `CARGO_ALLOW_REDACT_IDENTITY=1` to redact them. |

## Contract Change Rules

Expand Down
3 changes: 2 additions & 1 deletion docs/schemas/add.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@
"control_flow_not_analyzed",
"data_flow_not_analyzed",
"external_evidence_tools_not_invoked",
"repository_code_not_executed"
"repository_code_not_executed",
"source_text_in_identity_fields"
]
},
"scanner_limitation": {
Expand Down
3 changes: 2 additions & 1 deletion docs/schemas/common.v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"control_flow_not_analyzed",
"data_flow_not_analyzed",
"external_evidence_tools_not_invoked",
"repository_code_not_executed"
"repository_code_not_executed",
"source_text_in_identity_fields"
]
},
"scanner_limitation": {
Expand Down
3 changes: 2 additions & 1 deletion docs/schemas/doctor.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,8 @@
"control_flow_not_analyzed",
"data_flow_not_analyzed",
"external_evidence_tools_not_invoked",
"repository_code_not_executed"
"repository_code_not_executed",
"source_text_in_identity_fields"
]
},
"scanner_limitation": {
Expand Down
3 changes: 2 additions & 1 deletion docs/schemas/explain.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@
"control_flow_not_analyzed",
"data_flow_not_analyzed",
"external_evidence_tools_not_invoked",
"repository_code_not_executed"
"repository_code_not_executed",
"source_text_in_identity_fields"
]
},
"scanner_limitation": {
Expand Down
76 changes: 60 additions & 16 deletions docs/schemas/list.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@
"control_flow_not_analyzed",
"data_flow_not_analyzed",
"external_evidence_tools_not_invoked",
"repository_code_not_executed"
"repository_code_not_executed",
"source_text_in_identity_fields"
]
},
"contains": {
Expand Down Expand Up @@ -92,7 +93,11 @@
"inventory": {
"type": "object",
"additionalProperties": false,
"required": ["scope", "scanner", "source"],
"required": [
"scope",
"scanner",
"source"
],
"properties": {
"scope": {
"type": "string",
Expand Down Expand Up @@ -128,35 +133,59 @@
"additionalProperties": false,
"properties": {
"kind": {
"type": ["string", "null"],
"type": [
"string",
"null"
],
"description": "Applied --kind filter, or null when all governed kinds are included."
},
"family": {
"type": ["string", "null"],
"type": [
"string",
"null"
],
"description": "Applied --family filter, or null when all scanner or policy families are included."
},
"owner": {
"type": ["string", "null"],
"type": [
"string",
"null"
],
"description": "Applied --owner filter, or null when all policy owners are included."
},
"classification": {
"type": ["string", "null"],
"type": [
"string",
"null"
],
"description": "Applied --classification filter, or null when all policy classifications are included."
},
"path": {
"type": ["string", "null"],
"type": [
"string",
"null"
],
"description": "Applied --path source-tree path or path-prefix filter, or null when all scopes are included."
},
"source_package": {
"type": ["string", "null"],
"type": [
"string",
"null"
],
"description": "Applied --source-package filter, or null when all scanner-provided source package contexts are included."
},
"allow_id": {
"type": ["string", "null"],
"type": [
"string",
"null"
],
"description": "Applied --allow-id filter, or null when all durable allow IDs are included."
},
"status": {
"type": ["string", "null"],
"type": [
"string",
"null"
],
"enum": [
"matched",
"new",
Expand Down Expand Up @@ -210,7 +239,9 @@
"summary": {
"type": "object",
"additionalProperties": false,
"required": ["allow_entries"],
"required": [
"allow_entries"
],
"properties": {
"allow_entries": {
"type": "integer",
Expand Down Expand Up @@ -245,7 +276,8 @@
"control_flow_not_analyzed",
"data_flow_not_analyzed",
"external_evidence_tools_not_invoked",
"repository_code_not_executed"
"repository_code_not_executed",
"source_text_in_identity_fields"
]
},
"scanner_limitation": {
Expand Down Expand Up @@ -324,7 +356,10 @@
]
},
"family": {
"type": ["string", "null"]
"type": [
"string",
"null"
]
},
"owner": {
"type": "string"
Expand All @@ -336,7 +371,10 @@
"type": "string"
},
"source_package": {
"type": ["string", "null"],
"type": [
"string",
"null"
],
"description": "Scanner-provided source package context; not Cargo metadata or build proof."
},
"evidence_count": {
Expand All @@ -363,10 +401,16 @@
"description": "Whether this allow entry uses wildcard source-tree scope in path, glob, or selector glob."
},
"review_after": {
"type": ["string", "null"]
"type": [
"string",
"null"
]
},
"expires": {
"type": ["string", "null"]
"type": [
"string",
"null"
]
},
"reason": {
"type": "string"
Expand Down
Loading
Loading