Skip to content

fix(aws): redact secretsmanager get-secret-value payload#1986

Open
YOMXXX wants to merge 1 commit into
rtk-ai:developfrom
YOMXXX:fix/aws-secretsmanager-redact
Open

fix(aws): redact secretsmanager get-secret-value payload#1986
YOMXXX wants to merge 1 commit into
rtk-ai:developfrom
YOMXXX:fix/aws-secretsmanager-redact

Conversation

@YOMXXX
Copy link
Copy Markdown
Contributor

@YOMXXX YOMXXX commented May 20, 2026

Summary

filter_secrets_get previously emitted the raw SecretString into LLM-bound stdout via format!("Secret: {}", ...), and the two tests at aws_cmd.rs:2495-2526 codified this leak as expected behaviour. This PR redacts the value by default, exposes only structural metadata (name + length + SHA-256 prefix + JSON-key shape), and adds opt-in escape hatches plus a one-shot stderr warning so the behaviour change is visible.

Reproduction

# Before
$ rtk aws secretsmanager get-secret-value --secret-id prod/rds/app
Name: prod/rds/app
Secret: {"username":"admin","password":"secret123"}    # <-- secret value lands in the LLM context

# After (default)
$ rtk aws secretsmanager get-secret-value --secret-id prod/rds/app
rtk: redacted SecretString (set RTK_REVEAL_SECRETS=1 to disable)   # stderr, once per process
Name: prod/rds/app
SecretString: <redacted; 47 bytes; sha256=ab12cd34>
keys: [password, username]

# Opt-in (env)
$ RTK_REVEAL_SECRETS=1 rtk aws secretsmanager get-secret-value --secret-id prod/rds/app
Name: prod/rds/app
Secret: {"username":"admin","password":"secret123"}

# Opt-in (CLI flag, stripped before forwarding to the underlying `aws` binary)
$ rtk aws secretsmanager get-secret-value --reveal-secret --secret-id prod/rds/app
Name: prod/rds/app
Secret: {"username":"admin","password":"secret123"}

Root cause

src/cmds/cloud/aws_cmd.rs:1514-1541filter_secrets_get formats the raw SecretString into the LLM-bound output via format!("Secret: {}", ...). The two tests at lines 2495-2526 had codified the leak as expected behaviour.

Fix approach

  • Default behaviour: emit Name + a SecretString: <redacted; N bytes; sha256=XXXXXXXX> line + (for JSON-shaped secrets) a sorted keys: [...] line listing top-level keys. Value is never printed.
  • SHA-256 prefix: full SHA-256 of the secret string, hex-encoded, truncated to the first 8 hex characters. Enough to round-trip-verify between runs without exposing the value.
  • --reveal-secret flag: detected and stripped in the secretsmanager get-secret-value router branch (src/cmds/cloud/aws_cmd.rs) so the underlying aws binary never sees it. A process-local AtomicBool (REVEAL_SECRET_FLAG) signals the filter; it is cleared at end-of-invocation so subsequent in-process calls (tests, hooks) do not inherit it.
  • RTK_REVEAL_SECRETS=1: independent env-based opt-in checked by the filter directly.
  • One-shot warning: std::sync::Once guarantees the stderr line fires at most once per process even under parallel test execution. Tests are serialised via a private reveal_env_lock() Mutex (same pattern as tracking.rs::test_db_path_env_and_default).

Behaviour changes

  • Default output of rtk aws secretsmanager get-secret-value <id> no longer contains the secret value.
  • The two existing tests (test_filter_secrets_get, test_filter_secrets_get_plain_text) are updated to assert the new behaviour.
  • CHANGELOG entry recommended under Security for the next release.

No new dependencies (project already uses sha2). No public Rust API change. No unwrap() in production code.

Test plan

  • test_filter_secrets_get (modified — asserts redaction, no secret123/admin, contains sha256=)
  • test_filter_secrets_get_plain_text (modified — no plain-text-password)
  • test_filter_secrets_get_invalid_json (unchanged passthrough)
  • test_filter_secrets_get_emits_json_keys (new — JSON shape exposed, values do not)
  • test_filter_secrets_get_reveal_via_env (new — RTK_REVEAL_SECRETS=1 restores legacy output; ENV_LOCK-style mutex)
  • test_filter_secrets_get_reveal_via_flag (new — REVEAL_SECRET_FLAG (set by --reveal-secret) restores legacy output)
  • test_filter_secrets_get_token_savings (new — multi-line aws secretsmanager get-secret-value --output json fixture, ≥60% savings target)
  • cargo fmt --all clean
  • cargo clippy --all-targets zero warnings
  • cargo test --bin rtk -- --test-threads=8: 1905 passed / 0 failed

Refs #1875 (Finding 1)

filter_secrets_get previously emitted the raw SecretString into
LLM-bound stdout via `format!("Secret: {}", ...)`. Tests at
aws_cmd.rs:2495-2526 had codified this leak as expected behaviour.

This change redacts the SecretString by default and emits only
non-sensitive structural metadata:
- Name (the lookup key the agent asked for)
- SecretString length + sha256 prefix (round-trip verification)
- For JSON secrets, the set of top-level keys (shape, not values)

Two opt-in escape hatches restore the previous behaviour for the
rare-but-real "AWS-rotation shell script" workflow:
- RTK_REVEAL_SECRETS=1 (env)
- --reveal-secret (CLI flag, stripped before forwarding to aws)

A one-shot stderr warning fires once per process the first time
redaction is applied, so the change is not silent.

Refs rtk-ai#1875 (Finding 1)
quangdang46 added a commit to quangdang46/rust_token_cost_optimizer that referenced this pull request May 24, 2026
Issue: aws secretsmanager get-secret-value emitted the SecretString
verbatim into the agent's context. With the auto-rewrite hook in place
the LLM saw raw passwords, RDS connection strings, API keys etc. on
every call — and from there into chat logs / training corpora.

Fix:
- Default redact: emit '<redacted; N bytes; sha256=XXXXXXXX>' instead
  of the raw value. SHA-256 prefix is stable across runs so the agent
  can round-trip-verify it saw the same secret.
- For JSON-shaped secrets, expose top-level key names (sorted) so the
  agent knows the shape, e.g. 'keys: [password, port, username]', but
  no values leak.
- Two opt-out paths for legitimate use:
  * env: RTCO_REVEAL_SECRETS=1
  * CLI: rtco aws secretsmanager get-secret-value --reveal-secret …
- One-shot stderr warning the first time we redact in a process
  (std::sync::Once) so the user knows the fallback exists.

Tests (4 added, 3 updated):
- redaction is the default (test_filter_secrets_get,
  test_filter_secrets_get_plain_text)
- top-level JSON keys are emitted, values are not
  (test_filter_secrets_get_emits_json_keys)
- RTCO_REVEAL_SECRETS=1 restores legacy verbatim behaviour
  (test_filter_secrets_get_reveal_via_env)
- --reveal-secret flag has same effect via REVEAL_SECRET_FLAG
  (test_filter_secrets_get_reveal_via_flag)
- representative RDS-credential payload still hits >=60% token savings
  with redaction on (test_filter_secrets_get_token_savings)
- env-touching tests serialise via reveal_env_lock() Mutex so cargo
  test parallelism is safe.

Verified: cargo clippy --all-targets -D warnings clean,
cargo test 1932 passed / 0 failed (3 consecutive runs).

Ports rtk-ai/rtk#1986
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant