Skip to content

Redacting Debug for input types with secrets; constant-time MCP token check #848

Open

Description

@smart-ssh-planner

Goal

Input types that carry plaintext secrets cannot leak them through Debug (for example through a future tracing::debug!(?input)), and the local MCP server compares its bearer token in constant time and keeps it as a secret type.

Context

  • Input DTOs with plaintext secrets and #[derive(Debug)]:
    • crates/app-logic/src/dto.rs:344-357 AiProviderConfigInput.api_key: String
    • crates/app-logic/src/dto.rs:694-712 ServerInput.sudo_password: Option<String>
    • crates/app-logic/src/dto.rs:786-832 AuthMethodInput (value, key_content, passphrase, cert_content)
    • crates/app-shell/src/mcp_settings.rs:36-50 McpServerSettingsDto.token (output DTO; the token is shown to the user on purpose, but Debug must not print it)
      Today nothing logs these with ? (searched), so this is hardening, not a fix of a current leak.
  • Pattern already in the repo: Prepared in crates/core/src/outgoing.rs:66-73 with a hand-written Debug printing [REDACTED], tested at outgoing.rs:99-107. secrecy 0.10 is a dependency of core and app-logic.
  • crates/app-shell/src/commands/ai_providers.rs:203-211 copies the API key into a plain String (expose_secret().to_string()) that is never zeroed.
  • MCP auth: crates/mcp-server/src/auth.rs:20-75 keeps the token as Arc<Mutex<String>> (SharedToken) and compares with token == expected; the comment says constant time would be extra hardening that the spec does not require. The server binds to 127.0.0.1 (crates/mcp-server/src/config.rs:34). subtle 2.6 is already in Cargo.lock.
  • crates/core/src/credentials/mod.rs:1-15 module doc still describes "planned content"; the real implementation lives in crates/core/src/profiles/credentials.rs and crates/credentials-keyring.

Requirements

  • Each DTO above gets a Debug that prints secret fields as [REDACTED] (presence may be shown, e.g. Some([REDACTED])), either by a hand-written Debug or by changing the field type to SecretString with serde deserialization (secrecy's serde feature). Prefer SecretString where the field flows on into code that already expects a secret; keep the IPC JSON shape unchanged in both cases.
  • ai_providers.rs:203-211: keep the key as SecretString (or Zeroizing<String>) until the point where the HTTP client needs it.
  • SharedToken stores the token as SecretString, has no Debug that prints it, and require_bearer_token compares with subtle::ConstantTimeEq on the byte slices (a length mismatch may return early). The empty-token rule (issue Reject MCP requests when the expected token is empty #115) stays.
  • Update the outdated comment in auth.rs and the module doc of crates/core/src/credentials/mod.rs (point to where credentials actually live).
  • Add a short rule to AGENTS.md (or the security invariants file) next to the logging rules: "types that carry secrets have a redacting Debug".
  • Comments in new code in English.

Acceptance Criteria

  • Test per DTO (AiProviderConfigInput, ServerInput, every AuthMethodInput variant with a secret, McpServerSettingsDto): format!("{:?}", value) built from a test secret does not contain the test secret.
  • Counter-check: the same test with a plain #[derive(Debug)] would fail (state it in the PR, or keep a test that checks the non-secret fields are still printed).
  • Deserialisation tests for the changed DTOs: the existing IPC JSON (camelCase field names, null for absent values) still deserialises into the same values.
  • MCP auth tests (auth.rs tests: missing, wrong, correct, rotated, empty token) pass unchanged; a new test checks that a token differing only in the last byte and a token of different length are rejected.
  • grep -n "token == expected" crates/mcp-server returns nothing.
  • Relevant automated tests pass.

Technical Notes

  • Security-critical area (credentials, MCP authentication): read .claude/skills/smart-ssh-coder/references/security-invariants.md (section "Redaction und Geheimnisse") and ADR 0086 (content only at debug level). Review priority elevated.
  • No change to persisted data, IPC payloads or behaviour visible to the user.
  • tauri::command argument types must keep deserialising from the same JSON; scripts/check-tauri-commands.mjs must still pass.

Spec impact: none

Related Work

Activity

  1. added this to the 0.8.0 milestone on Oct 26, 2026
  2. 18 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Fields

    Priority

    P1 - High

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions