Skip to content

fix: reject secrets from caches and database parameters#162

Merged
joshcramer merged 1 commit into
mainfrom
feat/std-secrets
Jul 13, 2026
Merged

fix: reject secrets from caches and database parameters#162
joshcramer merged 1 commit into
mainfrom
feat/std-secrets

Conversation

@larimonious

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #161 from a late architecture-review result that arrived after the original PR merged.

  • reject secret-bearing cache_fetch() options before URL-only cache lookup or storage
  • make SQLite and PostgreSQL parameter conversion fallible and recursively reject Secret rather than persisting the redaction marker
  • add an explicit nested-secret regression for task/channel serialization
  • document the cache boundary in generated stdlib docs

Verification

  • cargo test --all-targets
  • cargo test --lib secret -- --nocapture
  • cargo run --quiet -- docs --generate
  • cargo run --quiet -- docs --validate
  • cargo build --profile dev-release
  • changed-line Clippy diagnostics: 0
  • focused Codex review: APPROVED, no P0-P2 findings

@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens secret-value boundaries across the HTTP cache, SQLite, and PostgreSQL integrations by rejecting Secret-bearing values before they can be persisted, cached, or serialized. The changes convert the previously infallible value_to_sql_param and value_to_sqlite helpers to fallible functions, update all three call sites in each module to propagate the error, and add an upfront contains_secret() guard in cache_fetch().

  • cache_fetch() now rejects any options map whose values (recursively) contain a Secret before touching the cache or the network, preventing credential-free cache keys from serving credentialed responses.
  • value_to_sql_param (PostgreSQL) and value_to_sqlite (SQLite) are now Result-returning; all three query/execute helpers in each module use collect::<Result<_>>()? to propagate the error.
  • A concurrent.rs regression test verifies that a Secret nested inside a Map is rejected at task/channel serialization boundaries, and new focused tests in each module confirm the canary string is never present in the returned error.

Confidence Score: 5/5

Safe to merge. All three secret-rejection guards fire before any sensitive data reaches a cache, database parameter, or serialized task boundary, and the recursive contains_secret() implementation ensures nested secrets at any depth are caught.

The logic change is narrow and well-contained: two helpers become fallible, three call sites per module are updated with collect::<Result<_>>()?, and an upfront guard is added in cache_fetch(). The contains_secret() implementation is recursive and covers Array, Map, Struct, and EnumValue, so no nesting depth bypasses the check. Each new code path has a focused test that also verifies the canary string does not surface in the error message. No existing behaviour is altered for non-secret values.

No files require special attention. The PostgreSQL Value::Array arm still uses format!() for non-String elements, but the upfront guard guarantees no secret ever reaches that branch.

Important Files Changed

Filename Overview
src/stdlib/http.rs Adds an upfront secret guard in cache_fetch() before any cache lookup or network call; guard correctly uses recursive contains_secret(); test verifies canary does not appear in the error message.
src/stdlib/postgres.rs Makes value_to_sql_param fallible with an upfront contains_secret() guard; all three call sites (pg_query, pg_query_one, pg_execute) propagate the error via collect::<Result<_>>()?; array handling inside the match still formats non-String elements with format!() but the guard fires first, preventing any secret from reaching that path.
src/stdlib/sqlite.rs Makes value_to_sqlite fallible with an upfront contains_secret() guard; all three call sites propagate via collect::<Result<_>>()?; test wraps a Secret inside a Map to exercise the recursive path.
src/stdlib/concurrent.rs Adds a regression test for nested secrets (Secret inside a Map) crossing task/channel serialization boundaries; canary assertion present.
docs/STDLIB_REFERENCE.md Updates the cache_fetch() doc comment to document the secret-rejection behaviour; matches the updated source comment in http.rs.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Intent Value as parameter] --> B{contains_secret recursive check}
    B -- Yes --> C[IntentError type_error returned\ncanary never in message]
    B -- No --> D[Proceed to call site]

    D --> E[cache_fetch http.rs]
    D --> F[value_to_sql_param postgres.rs]
    D --> G[value_to_sqlite sqlite.rs]
    D --> H[SerializedValue concurrent.rs]

    E --> E2[Cache lookup then network fetch then store]
    F --> F1[pg_query pg_query_one pg_execute\ncollect Result propagates error]
    G --> G1[sqlite_query sqlite_query_one sqlite_execute\ncollect Result propagates error]
    H --> H1[Task or Channel boundary crossing]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Intent Value as parameter] --> B{contains_secret recursive check}
    B -- Yes --> C[IntentError type_error returned\ncanary never in message]
    B -- No --> D[Proceed to call site]

    D --> E[cache_fetch http.rs]
    D --> F[value_to_sql_param postgres.rs]
    D --> G[value_to_sqlite sqlite.rs]
    D --> H[SerializedValue concurrent.rs]

    E --> E2[Cache lookup then network fetch then store]
    F --> F1[pg_query pg_query_one pg_execute\ncollect Result propagates error]
    G --> G1[sqlite_query sqlite_query_one sqlite_execute\ncollect Result propagates error]
    H --> H1[Task or Channel boundary crossing]
Loading

Reviews (1): Last reviewed commit: "fix: reject secrets from caches and data..." | Re-trigger Greptile

@joshcramer joshcramer merged commit 4f190c7 into main Jul 13, 2026
9 checks passed
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.

2 participants