Harden daemon: fail closed on symlinked COVEN_HOME and non-socket socket path#144
Merged
Merged
Conversation
The daemon created COVEN_HOME and bound coven.sock without checking whether either path was a symlink, or (for the socket) a non-socket file it would blindly remove. docs/AUTH.md lists exactly these as the remaining fail-closed hardening items before broad distribution. - ensure_private_coven_home now refuses a COVEN_HOME that is a symlink, so following it cannot redirect daemon state (socket, status, SQLite ledger) outside the trusted directory. - bind_api_socket now inspects the socket path with symlink_metadata and refuses a symlink or any non-socket file, instead of calling remove_file on whatever happens to be there; only a genuine stale socket is replaced. std-only, no new dependencies, no unsafe; Unix-gated. Adds three cfg(unix) tests. Full workspace suite green: fmt --check, clippy --all-targets -D warnings, cargo test --workspace --locked. The euid-ownership fail-closed check from AUTH.md is left as a follow-up; it needs a libc geteuid dependency. Refs: docs/AUTH.md "Current hardening gap"
Contributor
Author
|
/Lock-in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the two still-open
docs/AUTH.md"Current hardening gap" fail-closed checks incrates/coven-cli/src/daemon.rs:ensure_private_coven_homenow refuses aCOVEN_HOMEthat is a symlink, so following it cannot redirect daemon state (socket, status, SQLite ledger) outside the trusted directory.bind_api_socketnow inspects the socket path withsymlink_metadataand refuses a symlink or any non-socket file, instead of callingremove_fileon whatever happens to be there. Only a genuine stale socket is replaced.Why
docs/AUTH.mdlists these exact conditions under "Before broad distribution, Rust should fail closed when ...COVEN_HOMEresolves through a symlink ... an existing socket path is a symlink or non-socket file." The TypeScript plugin already validates the socket trust anchor; this brings the Rust daemon in line. The euid-ownership check from the same list is intentionally left out of this PR because it needs alibc::geteuiddependency.Changes
ensure_private_coven_home:symlink_metadataguard; bail when the home is a symlink.bind_api_socket:symlink_metadata+FileTypeExt::is_socket; bail on a symlink or non-socket; onlyremove_filea genuine stale socket; treatNotFoundas fresh.#[cfg(unix)]tests (symlinked home, symlinked socket path with target-not-deleted assertion, non-socket file at the socket path).std-only — no new dependencies, no
unsafe, Unix-gated.Verification
cargo fmt --check,cargo clippy --workspace --all-targets -- -D warnings, andcargo test --workspace --lockedall green on Linux: 696 unit + 4 smoke tests pass, including the 3 new ones, no regressions.Tracked by #142.
Note: I see PRs are frozen until July per CONTRIBUTING — opening this for visibility since it is a small, isolated security hardening that closes a documented gap. Completely fine to close or hold it until the window reopens; the branch will keep.