Skip to content

fix(audit): batched low-severity cleanup (F11, F12, F13, F14, F15)#35

Merged
MasonStation merged 1 commit intomainfrom
fix/audit-cleanup-f11-f15
Apr 26, 2026
Merged

fix(audit): batched low-severity cleanup (F11, F12, F13, F14, F15)#35
MasonStation merged 1 commit intomainfrom
fix/audit-cleanup-f11-f15

Conversation

@MasonStation
Copy link
Copy Markdown
Contributor

Summary

Closes five low-severity audit findings in a single batched PR because each change is small and independent:

  • F11 phantom-core/dotenv.rslooks_like_secret heuristic misses: add PWD, -----BEGIN , and URL-with-auth-in-query detection.
  • F12 phantom-core/dotenv.rsparse_str rewrite so double-quoted .env values can span multiple lines (PEM keys). Adds \n/\r/\t/\/\" escape handling inside "...".
  • F13 phantom-vault/keychain.rs — replace plaintext secret names in keychain metadata with a 64-bit SHA-256 of {project_id}:{name}. Lazy read-time migration keeps existing users' stored secrets reachable. New sha2 direct dep.
  • F14 phantom-vault/lib.rs — the silent keychain-> file-vault demotion now prints a WARNING to stderr with the reason and fallback path. New PHANTOM_REQUIRE_KEYCHAIN=1 opt-in to hard-fail instead.
  • F15 phantom-core/config.rs, sync.rs#[serde(deny_unknown_fields)] on PhantomConfig, PhantomMeta, ServiceConfig, CloudConfig, SyncTarget. A typo like patern now fails at load time with a clear error.

Why bundle

Each item is ≤ 100 LOC of change in a different module; splitting would be churn without review benefit. The full commit message has per-finding detail.

Test plan

  • cargo test --workspace — phantom-cli 1, phantom-core 43, phantom-mcp 10, phantom-proxy 27, phantom-vault 19, all pass
  • New tests:
    • F11: PWD, PEM armor header, URL-auth-in-query positive cases + plain URL negative case
    • F12: multi-line PEM parses as one entry; multi-line PEM classified as Secret; \n\t escapes; unterminated quote doesn't panic; single-quoted stays literal
    • F13: hash determinism, per-project distinctness, per-name distinctness, plaintext-name-not-substring, output format
    • F15: unknown top-level field rejected; typo on ServiceConfig (patern) rejected; valid config round-trip still works
  • cargo clippy --all-targets -- -D warnings clean
  • cargo fmt --check clean
  • F13 migration verified by construction: retrieve tries new hash, falls back to legacy name on NoEntry, re-stores at new location, deletes legacy. Best-effort cleanup in store/delete.

Compatibility notes

  • F15: if a user has an existing .phantom.toml with fields not in the schema, it will now fail to parse. Intentional — that's the whole point. The audit flagged patern as a realistic typo that would silently disable a protection.
  • F13: first-time reads of pre-F13 entries auto-migrate in place. No user action required.
  • F14: default behavior (fallback with warning) preserves CI/Docker workflows. PHANTOM_REQUIRE_KEYCHAIN=1 is opt-in.

🤖 Generated with Claude Code

Addresses five low-severity findings from the v0.5.1 adversarial audit in
a single batched PR because each change is small and independent:

F11: looks_like_secret heuristic false-negatives (phantom-core/dotenv.rs)
  - Add `PWD` to the key-name patterns (covers `DB_PWD=hunter2` and similar
    short forms that previously missed).
  - Add `-----BEGIN ` as a value prefix marker so PEM-armored keys land in
    the vault even when the var name is generic.
  - Detect URLs that carry auth in the query string
    (`https://host/?api_key=...`, `token=...`, `signature=...`), not just
    the `user:pass@host` userinfo form.

F12: multi-line .env values (phantom-core/dotenv.rs)
  - Rewrite `parse_str` to support double-quoted values that span multiple
    lines — typical for PEM-encoded private keys stored in `.env`.
  - Apply `\n` / `\r` / `\t` / `\` / `\"` / `\'` escape handling inside
    double-quoted values; single-quoted values stay literal.
  - Unterminated quote preserves the opening line as an unparsed `Other`
    line rather than panicking or silently dropping content.

F13: hash keychain entry names (phantom-vault/keychain.rs)
  - Replace the plaintext secret name in keychain service/account metadata
    with a 64-bit (16 hex char) SHA-256 of `{project_id}:{name}`. On Linux
    secret-service the plaintext name was queryable without unlock.
  - Introduce an `h-` prefix on new entry service strings so they're
    distinguishable from pre-F13 entries.
  - Lazy read-time migration: `retrieve(name)` checks the new hashed entry
    first; on NoEntry it falls back to the legacy plaintext entry, returns
    its value, and silently re-stores at the hashed location so future
    reads hit the new path. `store` and `delete` best-effort clean up any
    legacy entry for the name.
  - Add `sha2 =

Assisted-By: ashlr-plugin <https://plugin.ashlr.ai>"0.10"` as a direct phantom-vault dep. Add unit tests for
    the hash function's determinism, per-project/per-name distinctness,
    plaintext-name-not-substring property, and output format.

F14: surface keychain -> file-vault demotion (phantom-vault/lib.rs)
  - `create_vault` now prints a `WARNING` on stderr when it falls back
    from the OS keychain to the encrypted-file vault, including the real
    failure reason and the fallback path. The demotion is a security
    posture change (encrypted-file secrets are recoverable by anyone with
    passphrase + disk, vs. keychain's per-user unlock) and must not be
    silent.
  - Add `PHANTOM_REQUIRE_KEYCHAIN=1` opt-in env var. When set, fallback
    hard-fails with a non-zero exit and a clear error instead of demoting.
    Default is unchanged (fallback allowed with a visible warning) to
    preserve CI/Docker compatibility.

F15: deny_unknown_fields on .phantom.toml schema (phantom-core/config.rs)
  - Add `#[serde(deny_unknown_fields)]` to PhantomConfig, PhantomMeta,
    ServiceConfig, CloudConfig, and SyncTarget. A typo like `patern`
    (vs `pattern`) on a service config used to silently disable proxy
    routing; now it fails at load time with an error that names the bad
    field. Add tests covering both the top-level and service-config cases
    plus a round-trip test so our own output never trips the check.

All workspace tests pass (phantom-cli 1, phantom-core 43, phantom-mcp 10,
phantom-proxy 27, phantom-vault 19). clippy --all-targets -D warnings
clean; rustfmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@MasonStation MasonStation force-pushed the fix/audit-cleanup-f11-f15 branch from 307dabf to 2ac0a6d Compare April 26, 2026 15:43
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
web Ready Ready Preview, Comment Apr 26, 2026 3:44pm

Request Review

@MasonStation MasonStation merged commit f07f8ad into main Apr 26, 2026
4 of 6 checks passed
@MasonStation MasonStation deleted the fix/audit-cleanup-f11-f15 branch April 26, 2026 15:43
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.

1 participant