fix(log): scrub capability tokens from INFO/WARN logs #170
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| id: no-sensitive-log-fields | ||
| message: | | ||
| INFO/WARN/ERROR tracing fields must not carry capability material: token, | ||
| authorization, cookie, headers, body, payload, or signed_url. These leak | ||
| bearer material into journald and OTLP. Use operation/kind/size/result | ||
| fields instead. The conformance flow recorder (recording.rs) is exempt. | ||
| severity: error | ||
| language: rust | ||
| # recording.rs deliberately captures every header and body for conformance. | ||
| ignores: | ||
| - "**/recording.rs" | ||
|
Comment on lines
+10
to
+11
There was a problem hiding this comment. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Expect exactly one intended conformance recorder before narrowing the ignore.
fd -a '^recording\.rs$' .Repository: preloopdev/preloop Length of output: 221 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- rules/no-sensitive-log-fields.yml ---'
cat -n rules/no-sensitive-log-fields.yml
printf '%s\n' '--- crates/preloop-runner-server/src/recording.rs ---'
cat -n crates/preloop-runner-server/src/recording.rs
printf '%s\n' '--- references to recording.rs and no-sensitive-log-fields ---'
rg -n --hidden -g '!target' -g '!node_modules' 'recording\.rs|no-sensitive-log-fields|ignores:' .Repository: preloopdev/preloop Length of output: 10582 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- rules/no-sensitive-log-fields.yml ---'
cat -n rules/no-sensitive-log-fields.yml
printf '%s\n' '--- crates/preloop-runner-server/src/recording.rs ---'
cat -n crates/preloop-runner-server/src/recording.rs
printf '%s\n' '--- references ---'
rg -n --hidden -g '!target' -g '!node_modules' \
'recording\.rs|no-sensitive-log-fields|ignores:' .Repository: preloopdev/preloop Length of output: 10538 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- ignore patterns in neighboring rules ---'
cat -n rules/no-raw-secret-replace.yml
cat -n rules/no-expose-in-loop.yml
printf '%s\n' '--- rule-engine configuration and tests ---'
rg -n --hidden -g '!target' -g '!node_modules' \
'ignore|ignores|glob|rules/' Cargo.toml '**/Cargo.toml' '*.rs' '*.yml' '*.yaml' 2>/dev/null || true
printf '%s\n' '--- all Rust files and repository status ---'
git ls-files '*recording.rs'
git status --shortRepository: preloopdev/preloop Length of output: 1772 Scope the exemption to the conformance recorder. Replace 🤖 Prompt for AI Agents |
||
| rule: | ||
| any: | ||
| # info!(token, …) / warn!(authorization = …, …) | ||
| - pattern: info!($$$ARGS, token, $$$REST) | ||
| - pattern: warn!($$$ARGS, token, $$$REST) | ||
| - pattern: error!($$$ARGS, token, $$$REST) | ||
| - pattern: info!(token, $$$REST) | ||
| - pattern: warn!(token, $$$REST) | ||
| - pattern: error!(token, $$$REST) | ||
| - pattern: info!($$$ARGS, authorization, $$$REST) | ||
| - pattern: warn!($$$ARGS, authorization, $$$REST) | ||
| - pattern: error!($$$ARGS, authorization, $$$REST) | ||
| - pattern: info!($$$ARGS, cookie, $$$REST) | ||
| - pattern: warn!($$$ARGS, cookie, $$$REST) | ||
| - pattern: error!($$$ARGS, cookie, $$$REST) | ||
| - pattern: info!($$$ARGS, headers, $$$REST) | ||
| - pattern: warn!($$$ARGS, headers, $$$REST) | ||
| - pattern: error!($$$ARGS, headers, $$$REST) | ||
| - pattern: info!($$$ARGS, signed_url, $$$REST) | ||
| - pattern: warn!($$$ARGS, signed_url, $$$REST) | ||
| - pattern: error!($$$ARGS, signed_url, $$$REST) | ||
| # Assigned form: info!(token = value, …) — the shorthand patterns above | ||
| # do not match it, so a sensitive field could bypass the rule. | ||
| - pattern: info!($$$ARGS, token = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, token = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, token = $$$VALUE, $$$REST) | ||
| - pattern: info!($$$ARGS, authorization = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, authorization = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, authorization = $$$VALUE, $$$REST) | ||
| - pattern: info!($$$ARGS, cookie = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, cookie = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, cookie = $$$VALUE, $$$REST) | ||
| - pattern: info!($$$ARGS, headers = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, headers = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, headers = $$$VALUE, $$$REST) | ||
| - pattern: info!($$$ARGS, signed_url = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, signed_url = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, signed_url = $$$VALUE, $$$REST) | ||
| # Assigned body/payload in plain, ?-debug and %-display forms. | ||
| - pattern: info!($$$ARGS, body = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, body = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, body = $$$VALUE, $$$REST) | ||
| - pattern: info!($$$ARGS, ?body = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, ?body = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, ?body = $$$VALUE, $$$REST) | ||
| - pattern: info!($$$ARGS, %body = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, %body = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, %body = $$$VALUE, $$$REST) | ||
| - pattern: info!($$$ARGS, payload = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, payload = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, payload = $$$VALUE, $$$REST) | ||
| - pattern: info!($$$ARGS, ?payload = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, ?payload = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, ?payload = $$$VALUE, $$$REST) | ||
| - pattern: info!($$$ARGS, %payload = $$$VALUE, $$$REST) | ||
| - pattern: warn!($$$ARGS, %payload = $$$VALUE, $$$REST) | ||
| - pattern: error!($$$ARGS, %payload = $$$VALUE, $$$REST) | ||
| # ?body / body = … inside the macro | ||
| - pattern: info!(?body, $$$REST) | ||
| - pattern: warn!(?body, $$$REST) | ||
| - pattern: error!(?body, $$$REST) | ||
| - pattern: info!(body, $$$REST) | ||
| - pattern: warn!(body, $$$REST) | ||
| - pattern: error!(body, $$$REST) | ||
| - pattern: info!(%body, $$$REST) | ||
| - pattern: warn!(%body, $$$REST) | ||
| - pattern: error!(%body, $$$REST) | ||
| - pattern: info!(payload, $$$REST) | ||
| - pattern: warn!(payload, $$$REST) | ||
| - pattern: error!(payload, $$$REST) | ||
| - pattern: info!(?payload, $$$REST) | ||
| - pattern: warn!(?payload, $$$REST) | ||
| - pattern: error!(?payload, $$$REST) | ||
|
Contributor
There was a problem hiding this comment. 🟠 High The rule allows sensitive 🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
Comment on lines
+69
to
+84
There was a problem hiding this comment. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
probe="$(mktemp --suffix=.rs)"
trap 'rm -f "$probe"' EXIT
cat > "$probe" <<'RUST'
fn probe(body: String, payload: String) {
info!(operation = "upload", body, "request");
warn!(operation = "upload", ?body, "request");
error!(operation = "upload", %body, "request");
info!(operation = "upload", payload, "request");
warn!(operation = "upload", ?payload, "request");
error!(operation = "upload", %payload, "request");
}
RUST
# Expect one finding for each sensitive shorthand field.
ast-grep scan --rule rules/no-sensitive-log-fields.yml "$probe"Repository: preloopdev/preloop Length of output: 156 🏁 Script executed: set -euo pipefail
printf '%s\n' '--- rule file ---'
cat -n rules/no-sensitive-log-fields.yml
printf '%s\n' '--- repository references ---'
rg -n --glob '!target/**' 'no-sensitive-log-fields|recording\.rs|info!\(|warn!\(|error!\(' . | head -200Repository: preloopdev/preloop Length of output: 25949 🏁 Script executed: set -euo pipefail
probe="$(mktemp --suffix=.rs)"
trap 'rm -f "$probe"' EXIT
cat > "$probe" <<'RUST'
fn probe(body: String, payload: String) {
info!(operation = "upload", body, "request");
warn!(operation = "upload", ?body, "request");
error!(operation = "upload", %body, "request");
info!(operation = "upload", payload, "request");
warn!(operation = "upload", ?payload, "request");
error!(operation = "upload", %payload, "request");
}
RUST
printf '%s\n' '--- exact shorthand patterns ---'
for pattern in \
'info!(?body, $$$REST)' \
'info!($$$ARGS, ?body, $$$REST)' \
'error!(%payload, $$$REST)' \
'error!($$$ARGS, %payload, $$$REST)'
do
printf 'PATTERN: %s\n' "$pattern"
ast-grep run --lang rust --pattern "$pattern" "$probe" || true
done
printf '%s\n' '--- rule scan on an in-repository Rust file ---'
ast-grep scan --rule rules/no-sensitive-log-fields.yml crates/preloop-orchestrator/src/lib.rs || trueRepository: preloopdev/preloop Length of output: 542 🏁 Script executed: set -euo pipefail
probe="$(mktemp --suffix=.rs)"
trap 'rm -f "$probe"' EXIT
cat > "$probe" <<'RUST'
fn probe(body: String, payload: String) {
info!(operation = "upload", body, "plain body");
warn!(operation = "upload", ?body, "debug body");
error!(operation = "upload", %body, "display body");
info!(operation = "upload", payload, "plain payload");
warn!(operation = "upload", ?payload, "debug payload");
error!(operation = "upload", %payload, "display payload");
}
RUST
patterns=(
'info!($$$ARGS, body, $$$REST)'
'warn!($$$ARGS, ?body, $$$REST)'
'error!($$$ARGS, %body, $$$REST)'
'info!($$$ARGS, payload, $$$REST)'
'warn!($$$ARGS, ?payload, $$$REST)'
'error!($$$ARGS, %payload, $$$REST)'
'warn!($$$ARGS, ? $$$BODY, $$$REST)'
'warn!($$$ARGS, ?$BODY, $$$REST)'
)
for pattern in "${patterns[@]}"; do
printf '\nPATTERN: %s\n' "$pattern"
ast-grep run --lang rust --pattern "$pattern" "$probe" || true
done
printf '\n--- Rust CST for the debug shorthand invocation ---\n'
ast-grep run --lang rust --pattern 'warn!(operation = "upload", ?body, "debug body")' --debug-query=cst "$probe" || trueRepository: preloopdev/preloop Length of output: 1870 Match body and payload shorthand fields in every position. The shorthand patterns at lines 70–84 match only the first macro field. Add 🤖 Prompt for AI Agents |
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Log only an allowlisted result value.
result_hintcomes directly from the untyped request body and is logged beforeexecution_status_from_runner_resultvalidates it. A client can send arbitrary or large text inresult, which reintroduces untrusted request content into INFO logs.Normalize the value through the existing status mapping before logging. Emit fixed labels such as
renew,unknown, or the mapped status. Use the same sanitized label in the later result logs at Lines 345, 365, and 369.Proposed normalization
🤖 Prompt for AI Agents