fix(permissions): sandbox-symlink-fail-closed (Linux dangling-symlink fail-open) - #102
Merged
Merged
Conversation
… fail-open) SandboxContainmentChecker.FollowLinkToTarget relied only on catch(IOException) to fail closed on a dangling symlink. macOS/Windows throw and fail closed, but Linux returns the non-existent target without throwing, so a dangling in-sandbox symlink resolved to a non-null in-root path and the sandbox containment boundary auto-allowed the write (fail-open). Add an explicit Path.Exists(final) guard after re-anchoring so all platforms fail closed, and correct the backwards platform comment. Mirrors the tools-layer twin fixed in ac2e315 (#96). Adds cross-platform tests (dangling leaf inside sandbox, dangling ancestor inside sandbox, live in-sandbox link regression); the fix is only observable on Ubuntu CI since macOS already fails closed via the throw. Change: sandbox-symlink-fail-closed Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Why
SandboxContainmentChecker.FollowLinkToTarget(core/Dmon.Core/Permissions/) decides whether asandbox-mode write/edit/delete target is contained within the session'sassets/<session_id>/subtree. It is written to fail closed on a broken/dangling symlink, relying solely oncatch (IOException).That intent is defeated on Linux:
File.ResolveLinkTarget(returnFinalTarget: true)throws on macOS/Windows for a dangling link (catch fires →null→ reject) but returns the non-existent target on Linux without throwing. So a dangling in-sandbox symlink resolved to a non-null, non-existent in-root path, the checker treated it as contained, and the write was auto-allowed when it should be rejected — a fail-open in the sandbox containment boundary.This is the
Dmon.Coretwin of the tools-layerRealPathResolverbug already fixed inac2e315(#96), which explicitly left this file as out-of-scope follow-up. It is the last outstanding item from therepo-audit-2026-07-06follow-up batch.What changed
FollowLinkToTarget: added an explicit existence guard after the relative-path re-anchoring block —if (final is not null && !Path.Exists(final)) return null;— so a resolved-but-non-existent target fails closed on every platform. Corrected the backwards platform comment. No changes toResolveRealPath/ResolveExistingAncestor/IsSymlink/IsContained(they already treatnullas reject).Dmon.Core.Tests): dangling leaf inside sandbox → not contained; dangling ancestor inside → not contained; live in-sandbox link → still contained (over-broad-guard regression). Dangling links point inside the asset dir — the only construction where the unguarded Linux path resolves to a non-null in-root target (i.e. actually gates the fix).permission-model(cross-platform fail-closed-on-broken-symlink for sandbox asset-containment).Verification
make build— 0 warnings / 0 errors (TreatWarningsAsErrors)make test— green (Dmon.Core.Tests613 passed / 1 skipped)openspec validate sandbox-symlink-fail-closed --strict— validADR
ADR-006 (conservative permission model) — hardens existing documented fail-closed behaviour; no ADR conflict.
Change: sandbox-symlink-fail-closed
🤖 Generated with Claude Code