Skip to content

High: vault-resolved ACP auth_clients token can silently resolve to empty string, bypassing bearer auth #6270

Description

@bug-ops

Vulnerability

resolve_acp_auth_clients (src/acp.rs:48-105) resolves each [[acp.auth_clients]] entry's token_vault_key via vault.get_secret(key) and pushes the result straight into the client list with no non-emptiness check: Ok(Some(t)) => Some(t) (line 68), then clients.push(AcpClientToken { id: client.id.clone(), token }) (line 98-101) unconditionally once token is Some. If the vault secret resolves to an empty string, the resulting AcpClientToken has token: "".

AcpConfig::validate_auth_clients (crates/zeph-config/src/ui.rs:801-869) does reject empty/whitespace-only tokens — but only for the legacy auth_token field and inline client.token (lines 806-808, 851-856). Per its own doc comment, it cannot check token_vault_key values because "the vault is not unlocked at config-load time" — so this path has no non-emptiness check anywhere in the codebase, unlike the two paths validation does cover.

Downstream, zeph-acp's own bearer-auth middleware (crates/zeph-acp/src/transport/auth.rs, match_client / BearerAuthLayer) does its own independent hashed constant-time comparison — it does not go through the shared AuthConfig/auth_middleware primitive fixed by #6268, so that fix does not cover this path. match_client (line 73-79) hashes whatever token string the client was constructed with, including "", and finds the first client whose hash matches. A request with Authorization: Bearer (empty presented token) hashes to blake3::hash(b""), which matches a token: "" client and is authenticated as that client — TokenIdentity is injected and the request proceeds.

This also feeds the pre-bind guard: acp_has_auth_configured = app.config().acp.auth_token.is_some() || !app.config().acp.auth_clients.is_empty() (src/serve/mod.rs:232-233) only checks whether an auth_clients entry exists at all, not whether its (still-vault-unresolved-at-that-point) token is non-empty, so check_acp_auth_guard (src/serve/mod.rs:385-395) does not refuse a non-loopback bind in this scenario either.

Severity

High (P1) — same defect class as #6268 (empty-string secret silently produces a fully authenticated bypass state), but a distinct code path: zeph-acp's multi-client bearer auth does not consume zeph_common::http_middleware::AuthConfig, so the #6268 fix will not close this one. Not Critical: it requires a specific precondition (a token_vault_key-configured client whose vault secret resolves to an empty string — e.g. a placeholder/rotated-out vault entry) rather than being reachable from network input alone, and only affects the newer [[acp.auth_clients]] (#5868) multi-client feature, not the legacy [acp] auth_token field (which the existing config validation already protects).

Location

  • src/acp.rs:48-105 (resolve_acp_auth_clients, root cause — no .filter(|t| !t.trim().is_empty()) on the vault-resolved branch)
  • crates/zeph-config/src/ui.rs:801-869 (validate_auth_clients — explicitly cannot cover this branch, by design, since the vault isn't unlocked at config-load time)
  • crates/zeph-acp/src/transport/auth.rs:73-79 (match_client — hashes and compares an empty token like any other)
  • src/serve/mod.rs:232-233 (acp_has_auth_configured pre-bind check does not account for a client's resolved token being empty)

Remediation

  1. In resolve_acp_auth_clients, treat an empty/whitespace-only vault-resolved token the same as a missing one: Ok(Some(t)) if !t.trim().is_empty() => Some(t), else warn-and-skip (mirroring the existing Ok(None) "vault key not found; client disabled" branch).
  2. Consider a defense-in-depth check inside zeph-acp's BearerAuthLayer::new (or AcpClientToken construction) to reject/skip empty-token clients at the crate boundary, so any future caller of the crate gets the same protection without relying on src/acp.rs alone.

References

CWE-287 (Improper Authentication) / CWE-306. Same defect class as #6268 (different code path — zeph-acp's own bearer-auth implementation, not the shared AuthConfig/auth_middleware primitive).

Metadata

Metadata

Assignees

Labels

P1High ROI, low complexity — do next sprintacpAgent Client Protocol integrationbugSomething isn't workingsecuritySecurity-related issue

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions