Skip to content

Entry classification is never auto-elevated from detected token patterns, so sealing condition is always false #202

@danieljustus

Description

@danieljustus

Summary

The sealing decision in internal/mcp/server/tools_get.go:161 is gated on entry.Classification >= taint.Secret. But nowhere in the codebase is entry.Classification actually set to taint.Secret based on the secret's nature — it stays at its default (Public, the iota-zero value).

The vault does detect token type via DetectSecretType (internal/vault/types.go:98-149) — it recognizes ghp_*, github_pat_*, gho_*, ghs_*, ghr_*, AWS access keys, SSH keys, certificates, JWTs, basic auth, TOTP seeds. But the return value is a SecretType label only; it never feeds into entry.Classification.

Grep confirms this — the only assignment site for Classification (other than the type definition and a copy constructor) is in tests:

internal/vault/entry.go:34:   Classification taint.Classification \`json:\"classification,omitempty\"\`
internal/vault/entry.go:616:    Classification: entry.Classification,   // copy-through

Net effect: even if #201 (AutoUnseal default) is fixed, the sealed-response path will still not trigger for a brand-new `github/personal-access-token` entry, because its classification is `Public` and `Public < Secret`.

Impact

  • The sealing protection in tools_get.go:211-228 (sealEntryResponse) is effectively dead code for the default user flow.
  • Users who store a GitHub PAT, AWS key, etc. via the normal CLI or import path get no automatic protection — they would have to manually set classification: secret in YAML or via a hypothetical edit flow.

Proposed fix

  1. When an entry is created or updated, run a classification-elevation step:
    • If any field value matches a known high-risk pattern (the existing DetectSecretType patterns minus password / custom), set entry.Classification = max(current, taint.Secret).
    • Optionally: SSH private keys and AWS keys → taint.Restricted (one tier higher).
  2. Add openpass migrate classify (or a one-shot at vault open) to retro-classify existing entries.
  3. Allow users to override the inferred classification explicitly (down or up) — auto-detection should never lower a manually set classification.
  4. Test: round-trip a PAT through set_entry_fieldget_entry_value and assert the response is sealed.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: urgentsecuritySecurity vulnerability or hardening

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions