Repository navigation
fix(providers): redact the host context in CatalogLimitsRequest Debug - #145
Conversation
…tput Add a test asserting that formatting a catalog limits request for debugging keeps the model name but replaces the host context with a placeholder, so secrets are not leaked into logs. Auto-committed-on: cuttlefish Co-authored-by: Medulla <medulla@tinyhumans.ai>
… output Replace the derived Debug implementation on CatalogLimitsRequest with a manual one that prints the model name but masks the host context, so debug logging no longer leaks caller-supplied context values. Auto-committed-on: cuttlefish Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Warning Review limit reached
This review includes 2 billable files and costs up to $0.50. Or wait 51 minutes for your next included review. View limit details
CodeRabbit Verify
|
Tiny Sweeper reviewReviewed head: Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): fix(providers): redact the host context in CatalogLimitsRequest Debug. This pull request, titled "fix(providers): redact the host context in CatalogLimitsRequest Debug", is a follow-up to a prior change that merged before this fix was pushed, and it addresses two review threads from that work. Its stated purpose is to stop `CatalogLimitsRequest<Ctx>` from printing the host context via a derived `Debug`, which could expose an API key or session token held in that context. The PR replaces the derive with a hand-written `Debug` that prints `model` and a fixed `<host context>` placeholder, matching the existing behavior of the sibling request types, and adds a regression test in the integration test file. The only intended public API change is the `Debug` output of `CatalogLimitsRequest`. No trusted base-to-head diff was available, so the latest push delta is unavailable and nothing is characterized as a push delta. Review snapshot
Completeness: Complete What changed`CatalogLimitsRequest<Ctx>` in `crates/tinyinference-llm/src/providers/overrides.rs` no longer derives `Debug`; a manual `std::fmt::Debug` impl prints the `model` field and renders `context` as the literal `"<host context>"`, so a host context containing credentials is not emitted by `{:?}`. Because the impl no longer requires `Ctx: Debug`, `Debug` becomes available for more `Ctx` types, which the critique and tests lanes characterize as a relaxation rather than a break. The description lane notes this mirrors the four sibling request types. A new integration test, `catalog_limits_request_debug_redacts_the_host_context` in `crates/tinyinference-llm/tests/provider_overrides.rs`, builds a request with a synthetic secret in `context` and asserts the model is printed, the placeholder is printed, and the secret is absent. Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
Follow-up to #144, which merged before this fix was pushed. It addresses the two review threads there (Codex P1, CodeRabbit).
Cause:
CatalogLimitsRequest<Ctx>derivedDebug, so formatting it printed the host context with the host's ownDebug, including any API key or session token in it. The sibling request types (CatalogRequest,EmbeddingRequest,ReadinessRequest,ChatModelRequest) already print the context as<host context>.Fix: a hand-written
Debugthat printsmodeland redactscontext, matching the siblings.Regression test:
catalog_limits_request_debug_redacts_the_host_contextintests/provider_overrides.rs. It failed before the fix and passes after.Public API:
Debugoutput ofCatalogLimitsRequestchanges. Nothing else.Validation:
cargo fmt --all -- --checkok;cargo clippy --all-targets --all-features -- -D warningsok;cargo test -p tinyinference-llm --test provider_overrides: 4 passed.