allod/archetypes

Watch

Fork 0

Refuse a site root that is itself a symlink #63

Merged

vnprc merged 3 commits from agent/symlinked-site-root into master

2026-09-02 01:46:50 +01:00

allod-agent commented

2026-09-01 14:57:47 +01:00

Member

A site root that is itself a symlink is now refused, so a derivation whose output is ln -s /etc $out can no longer pass the build and publish the target from a public web root. Nothing else changes: a symlink inside the root is refused exactly as before, a root with internal links is accepted exactly as before, and no existing site or fixture behaves differently. What proves it is a new fixture that fails when the rule is removed. If it is wrong, revert the commit; the rule only ever refuses.

The defect

checks/site-symlink-escape.sh refused a symlink inside the site root and accepted a root that was a symlink. Three things combined, and the middle one is the surprise:

the root is resolved with readlink -f into site_root_real;
find "$site_root" -type l reports the root itself when the root is a link;
each found link's resolved target is compared against site_root_real, and for that root entry the two are equal by construction, so it was classified as inside.

The [[ ! -d $site_root ]] guard did not catch it either, because -d follows the link.

Reproduced before fixing:

mkdir -p real && ln -s /etc root-is-link && ln -s /etc real/leak
SITE_ROOT=$PWD/root-is-link bash checks/site-symlink-escape.sh   # exit 0 — accepted
SITE_ROOT=$PWD/real         bash checks/site-symlink-escape.sh   # exit 1 — refused

Caddy's file_server resolves symlinks at request time against the running host, so the accepted case answers GET /passwd from /etc. Nothing upstream stops it: types.pathInStore is lexical, and the module's context assertion is satisfied by a derivation whose output is a link.

The fix

[[ -L $site_root ]] is refused before the directory test, since -d follows links and would otherwise pass first.

Refused rather than judged, because there is nothing to judge it against: a link inside the tree is measured against the root that encloses it, and the root has no enclosing tree. A generator that wants to serve a directory can hand over the directory.

-L tests the final component only. An intermediate component being a symlink is deliberately not refused, and that is not an oversight: readlink -f resolves it, find walks the resolved tree, and every link inside is compared against that same resolved root, so the tree served and the tree scanned are the same one. The reasoning is in the file.

Risk

R1. The change only ever adds a refusal, and the refused shape is one no current site or fixture produces — the gate passes unchanged apart from the new case. The one way this could bite is a generator that legitimately emits a symlinked root; that generator now fails the build with a message naming exactly what to do instead, rather than silently publishing a host path.

Validation

./check.sh passes: all 31 steps.

The new fixture is proven non-vacuous: replacing the -L test with if false turns site-content red with site-symlink-escape accepted a site root that is itself a symlink. That absence of a fixture is why the defect survived, so the fixture is the substance of this change rather than decoration.

Also checked by hand, against the fixed script: a symlink inside a real root is still refused; a clean root is still accepted; a root containing an internal symlink is still accepted.

README.md claimed the system build refuses a symlink escaping the root, which was false for this shape; it now says what is actually enforced.

Closes allod/archetypes#61

A site root that is itself a symlink is now refused, so a derivation whose output is `ln -s /etc $out` can no longer pass the build and publish the target from a public web root. Nothing else changes: a symlink inside the root is refused exactly as before, a root with internal links is accepted exactly as before, and no existing site or fixture behaves differently. What proves it is a new fixture that fails when the rule is removed. If it is wrong, revert the commit; the rule only ever refuses. ## The defect `checks/site-symlink-escape.sh` refused a symlink *inside* the site root and accepted a root that *was* a symlink. Three things combined, and the middle one is the surprise: - the root is resolved with `readlink -f` into `site_root_real`; - `find "$site_root" -type l` reports the root itself when the root is a link; - each found link's resolved target is compared against `site_root_real`, and for that root entry the two are equal by construction, so it was classified as inside. The `[[ ! -d $site_root ]]` guard did not catch it either, because `-d` follows the link. Reproduced before fixing: ``` mkdir -p real && ln -s /etc root-is-link && ln -s /etc real/leak SITE_ROOT=$PWD/root-is-link bash checks/site-symlink-escape.sh # exit 0 — accepted SITE_ROOT=$PWD/real bash checks/site-symlink-escape.sh # exit 1 — refused ``` Caddy's `file_server` resolves symlinks at request time against the running host, so the accepted case answers `GET /passwd` from `/etc`. Nothing upstream stops it: `types.pathInStore` is lexical, and the module's context assertion is satisfied by a derivation whose output is a link. ## The fix `[[ -L $site_root ]]` is refused before the directory test, since `-d` follows links and would otherwise pass first. Refused rather than judged, because there is nothing to judge it against: a link inside the tree is measured against the root that encloses it, and the root has no enclosing tree. A generator that wants to serve a directory can hand over the directory. `-L` tests the final component only. An intermediate component being a symlink is deliberately not refused, and that is not an oversight: `readlink -f` resolves it, `find` walks the resolved tree, and every link inside is compared against that same resolved root, so the tree served and the tree scanned are the same one. The reasoning is in the file. ## Risk R1. The change only ever adds a refusal, and the refused shape is one no current site or fixture produces — the gate passes unchanged apart from the new case. The one way this could bite is a generator that legitimately emits a symlinked root; that generator now fails the build with a message naming exactly what to do instead, rather than silently publishing a host path. ## Validation `./check.sh` passes: all 31 steps. The new fixture is proven non-vacuous: replacing the `-L` test with `if false` turns `site-content` red with `site-symlink-escape accepted a site root that is itself a symlink`. That absence of a fixture is why the defect survived, so the fixture is the substance of this change rather than decoration. Also checked by hand, against the fixed script: a symlink inside a real root is still refused; a clean root is still accepted; a root containing an internal symlink is still accepted. `README.md` claimed the system build refuses a symlink escaping the root, which was false for this shape; it now says what is actually enforced. Closes allod/archetypes#61

allod-agent added 1 commit

2026-09-01 14:57:47 +01:00

Refuse a site root that is itself a symlink 47667d1998

allod-agent added 1 commit

2026-09-01 15:31:46 +01:00

Require the site root to resolve to itself, not merely to have a non-link last component 0559533d27

allod-agent commented

2026-09-01 15:32:06 +01:00

Author

Member

Review round 1 — the first fix was bypassable

An independent review of 47667d1 found that the rule it added could be walked past, and that a claim I had written into the file was wrong. Both reproduced here before acting. Fixed in 0559533.

High, fixed — a trailing slash bypassed the whole rule. [[ -L $site_root ]] tests the path as written, and Bash follows a trailing slash before evaluating -L. So the test became false, -d became true, and the symlinked root was accepted:

root      exit=1     (refused, as intended)
root/     exit=0     (accepted)
root/.    exit=0     (accepted)

This is reachable from the module, not just from the shell: lib.types.pathInStore accepts all three spellings — verified — so siteRoot = "${drv}/" where the derivation output is a link went straight through. The fixture only exercised the bare spelling, which is why the first round of this PR did not catch it.

High, fixed — my claim about intermediate symlinks was wrong. The file asserted that a symlink partway along the path is harmless because "readlink -f resolves it, find walks the resolved tree, so the tree served and the tree scanned are the same one." That holds only within one mount namespace at one instant. This scan runs inside a build sandbox; Caddy resolves at request time on the host. The reviewer demonstrated the mechanism directly — a file created after validation became visible through the same configured path.

The rule is now the one the issue proposed first. allod/archetypes#61's "Fix shape" section offered two options and I took the weaker one. It now refuses a root that does not resolve to itself: realpath -ms canonicalises ., .. and trailing slashes without following any link, and is compared against the fully resolved path, so a link at any position is caught. That closes the trailing-slash bypass and the intermediate-component case with one test instead of two.

Low, fixed — the diagnostic overclaimed. A root that is a link to a regular file, or a dangling link, is not a servable directory at all, and the old wording told the reader it "publishes a path outside the site". The message is now neutral: Site root must not be reached through a symlink: <lexical> resolves to <real>.

Fixtures

Four refusal spellings (root, root/, root/., and a real directory behind a symlinked parent) and four acceptance spellings (dir, dir/, dir/., dir/../dir), so the rule is pinned as being about symlinks rather than about path spelling.

Reverting the rule to the -L form the review defeated fails exactly three of them:

site-symlink-escape accepted a root reached through a symlink: …/root-is-symlink/
site-symlink-escape accepted a root reached through a symlink: …/root-is-symlink/.
site-symlink-escape accepted a root reached through a symlink: …/root-parent-link/site
3 site-content fixture(s) did not behave as claimed

Checked, not a defect

symlinkJoin and buildEnv produce a real directory containing links, not a link, so the root rule does not break either idiom — confirmed by building one and inspecting it. Their contents point outside the output and are refused by the pre-existing internal-link rule, identically before and after this change, so that is long-standing behaviour rather than anything this PR introduces. A symlinkJoined site root has never been servable.

Validation

./check.sh passes: all 31 steps.

## Review round 1 — the first fix was bypassable An independent review of `47667d1` found that the rule it added could be walked past, and that a claim I had written into the file was wrong. Both reproduced here before acting. Fixed in `0559533`. **High, fixed — a trailing slash bypassed the whole rule.** `[[ -L $site_root ]]` tests the path as written, and Bash follows a trailing slash before evaluating `-L`. So the test became false, `-d` became true, and the symlinked root was accepted: ``` root exit=1 (refused, as intended) root/ exit=0 (accepted) root/. exit=0 (accepted) ``` This is reachable from the module, not just from the shell: `lib.types.pathInStore` accepts all three spellings — verified — so `siteRoot = "${drv}/"` where the derivation output is a link went straight through. The fixture only exercised the bare spelling, which is why the first round of this PR did not catch it. **High, fixed — my claim about intermediate symlinks was wrong.** The file asserted that a symlink partway along the path is harmless because "`readlink -f` resolves it, `find` walks the resolved tree, so the tree served and the tree scanned are the same one." That holds only within one mount namespace at one instant. This scan runs inside a build sandbox; Caddy resolves at request time on the host. The reviewer demonstrated the mechanism directly — a file created after validation became visible through the same configured path. **The rule is now the one the issue proposed first.** `allod/archetypes#61`'s "Fix shape" section offered two options and I took the weaker one. It now refuses a root that does not resolve to itself: `realpath -ms` canonicalises `.`, `..` and trailing slashes without following any link, and is compared against the fully resolved path, so a link at *any* position is caught. That closes the trailing-slash bypass and the intermediate-component case with one test instead of two. **Low, fixed — the diagnostic overclaimed.** A root that is a link to a regular file, or a dangling link, is not a servable directory at all, and the old wording told the reader it "publishes a path outside the site". The message is now neutral: `Site root must not be reached through a symlink: <lexical> resolves to <real>`. ## Fixtures Four refusal spellings (`root`, `root/`, `root/.`, and a real directory behind a symlinked parent) and four acceptance spellings (`dir`, `dir/`, `dir/.`, `dir/../dir`), so the rule is pinned as being about symlinks rather than about path spelling. Reverting the rule to the `-L` form the review defeated fails exactly three of them: ``` site-symlink-escape accepted a root reached through a symlink: …/root-is-symlink/ site-symlink-escape accepted a root reached through a symlink: …/root-is-symlink/. site-symlink-escape accepted a root reached through a symlink: …/root-parent-link/site 3 site-content fixture(s) did not behave as claimed ``` ## Checked, not a defect `symlinkJoin` and `buildEnv` produce a real directory containing links, not a link, so the root rule does not break either idiom — confirmed by building one and inspecting it. Their *contents* point outside the output and are refused by the pre-existing internal-link rule, identically before and after this change, so that is long-standing behaviour rather than anything this PR introduces. A `symlinkJoin`ed site root has never been servable. ## Validation `./check.sh` passes: all 31 steps.

allod-agent added 1 commit

2026-09-01 15:49:46 +01:00

Refuse newline and .. components, which defeated the resolve-to-itself comparison c87d68ccf7

allod-agent commented

2026-09-01 15:50:06 +01:00

Author

Member

Review round 2 — two more bypasses, both reproduced

A second review, by a different model, found two further ways past the rule added in 0559533. Both reproduced here before acting. Fixed in c87d68c.

High, fixed — a symlink traversed before a ... build/link/../site with link -> current was accepted: realpath -ms folds link/.. away lexically, readlink -f follows the link and then applies .., and when the link points to a sibling both land on the same string. The comparison saw them equal. The path still went through a symlink, which is the thing being refused. lib.types.pathInStore permits .. inside the store, so the option type did not stop it either.

Medium, fixed — a trailing newline in the root name scanned a different directory. Every value here travels through $(...), which strips trailing newlines, so a root named site<newline> was reduced to the sibling site; that sibling was walked and pronounced clean while the requested directory held an escaping link. Reproduced at exit 0.

The fix, and why it is at the root rather than at the two cases

Both are refused by name. No site root needs a newline or a .. component, and refusing them is what makes the remaining comparison sound rather than lucky: with .. gone, realpath -ms only normalises . and repeated slashes, neither of which interacts with symlinks, so lexical-versus-resolved genuinely detects a link at any position. Patching each spelling as it was found would have left the comparison resting on the same weak footing.

That is the honest summary of three rounds on this file: -L was too narrow, resolve-to-itself was sound only for paths with no .., and the rule is now resolve-to-itself over a path shape that cannot defeat it. Each round found a genuinely new bypass rather than re-arguing the last one, but the direction of travel is worth naming — if a fourth appears, the answer is to accept only a plain absolute path with no special components at all rather than to add a fourth clause.

The suspected finding, answered

The review noted that comparing lexical against resolved would reject every store path on a host where /nix is itself a symlink, and did not reproduce it. It cannot arise where this runs: both callers execute inside a Nix build sandbox, where /nix/store is bind-mounted at its canonical path regardless of the host's layout. Confirmed by probing realpath -ms /nix/store against readlink -f /nix/store from inside a build, not by reasoning about it. The assumption is now recorded in the file, as the review asked.

Fixtures

Two new refusal cases — the ..-after-symlink shape and the newline-named root — and one correction: dir/../dir was an accepted spelling in the round-1 fixtures and is now refused, so that case moved rather than being left to contradict the rule.

Each proven non-vacuous:

dropping the .. refusal → accepted a root reached through a symlink: …/dotdot/link/../site
dropping the newline refusal → accepted a site root whose name contains a newline

Validation

./check.sh passes: all 31 steps.

## Review round 2 — two more bypasses, both reproduced A second review, by a different model, found two further ways past the rule added in `0559533`. Both reproduced here before acting. Fixed in `c87d68c`. **High, fixed — a symlink traversed before a `..`.** `build/link/../site` with `link -> current` was accepted: `realpath -ms` folds `link/..` away lexically, `readlink -f` follows the link and then applies `..`, and when the link points to a sibling both land on the same string. The comparison saw them equal. The path still went through a symlink, which is the thing being refused. `lib.types.pathInStore` permits `..` inside the store, so the option type did not stop it either. **Medium, fixed — a trailing newline in the root name scanned a different directory.** Every value here travels through `$(...)`, which strips trailing newlines, so a root named `site<newline>` was reduced to the sibling `site`; that sibling was walked and pronounced clean while the requested directory held an escaping link. Reproduced at exit 0. ## The fix, and why it is at the root rather than at the two cases Both are refused by name. No site root needs a newline or a `..` component, and refusing them is what makes the remaining comparison sound rather than lucky: with `..` gone, `realpath -ms` only normalises `.` and repeated slashes, neither of which interacts with symlinks, so lexical-versus-resolved genuinely detects a link at any position. Patching each spelling as it was found would have left the comparison resting on the same weak footing. That is the honest summary of three rounds on this file: `-L` was too narrow, resolve-to-itself was sound only for paths with no `..`, and the rule is now resolve-to-itself over a path shape that cannot defeat it. Each round found a genuinely new bypass rather than re-arguing the last one, but the direction of travel is worth naming — if a fourth appears, the answer is to accept only a plain absolute path with no special components at all rather than to add a fourth clause. ## The suspected finding, answered The review noted that comparing lexical against resolved would reject every store path on a host where `/nix` is itself a symlink, and did not reproduce it. It cannot arise where this runs: both callers execute inside a Nix build sandbox, where `/nix/store` is bind-mounted at its canonical path regardless of the host's layout. Confirmed by probing `realpath -ms /nix/store` against `readlink -f /nix/store` from inside a build, not by reasoning about it. The assumption is now recorded in the file, as the review asked. ## Fixtures Two new refusal cases — the `..`-after-symlink shape and the newline-named root — and one correction: `dir/../dir` was an accepted spelling in the round-1 fixtures and is now refused, so that case moved rather than being left to contradict the rule. Each proven non-vacuous: - dropping the `..` refusal → `accepted a root reached through a symlink: …/dotdot/link/../site` - dropping the newline refusal → `accepted a site root whose name contains a newline` ## Validation `./check.sh` passes: all 31 steps.