Skip to content

Hardening opportunities from an independent security review #59

Description

@ctrl-alt-automate

An independent full-codebase security review at commit 84d49a9 (six parallel passes: exfiltration surface, vault crypto, session broker, injection tiers, supply chain, secret-leakage hygiene) found no malicious behavior and no exfiltration path — the only network code in the product is the download-only, user-invoked updater, dependencies match the public checksum transparency log, and the fail-closed discipline across grant/consent/mount gates held up under adversarial review.

The review did surface a handful of preventive hardening opportunities. None are exploitable secret-disclosure paths (anything in that category was reported through the private channel per SECURITY.md); these are defense-in-depth items:

  1. Pin the goreleaser binary in the release job. Actions are SHA-pinned, but goreleaser-action is told version: "~> v2" (.github/workflows/ci.yml:152-171, release.yml:94-99), so the job holding MACOS_SIGN_P12 and the notary key executes whatever the latest goreleaser v2.x is at run time. The repo's own reasoning for SHA-pinning actions ("a tag is mutable") applies to this one remaining floating executable. An exact version + checksum would close it.

  2. Offer a re-seal path for legacy v1 envelopes. v1 (envelopeVersionAADLess) payloads decrypt with aad = nil forever (internal/vault/vault.go:238-240), so the ciphertext-swap scenario the envelopeAAD comment describes (envelope.go:139-142) remains possible between two v1 files for an attacker with vault-dir write access. RewrapAll rewraps only the DEK, so even a full MEK rotation never upgrades a v1 payload — only a value rotation does. A jit vault doctor nudge ("N secrets still carry v1 envelopes") or an opportunistic re-seal on Get would retire the format.

  3. Create jit scan --output reports 0600. os.Create(scanOutput) (internal/cli/scan.go:249) yields 0644 under the default umask. The report holds masked previews only (first 4 chars + key names + paths), but 0600 would match the posture everywhere else.

  4. Add a strength floor (or warning) for the export passphrase. internal/cli/vault.go:1720-1722 accepts any non-empty passphrase; Argon2id at 64 MiB/t=3 is a good KDF but cannot save a 1-character passphrase protecting a full-vault export.

  5. Validate Xucred.Version in the peercred check. internal/agent/peercred.go:35-47 checks only .Uid; comparing xucred.Version == unix.XUCRED_VERSION is a cheap belt-and-suspenders guard that the struct layout is what the code assumes.

  6. Validate MEK length on Keychain fetch. internal/keychainwrap/keychainwrap.go:228 accepts the item as-is; a corrupted/wrong-length item currently surfaces as a generic aes.NewCipher error instead of a clear "master key item is malformed" diagnostic.

Also noted while reviewing (no action asked, recorded for completeness): the tree-grant name filter matching on argv[0] within the anchored tree, and the audit-log entropy floor for weak unshaped secrets, are both already documented in-code as accepted tradeoffs; the CLAUDE.md SPDX count (364/364) has drifted from the actual 390/390.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions