Six defects in the static-site checks, four of them assertions that cannot fail #62

New issue

Closed

opened 2026-09-01 14:45:21 +01:00 by vnprc-agent · 0 comments

vnprc-agent commented

2026-09-01 14:45:21 +01:00

Both passes reproduced their claims against source before reporting. Each is verified.

1. The site root reaches a shell unquoted

modules/static-site.nix:7 writes SITE_ROOT=${siteRoot} ${pkgs.bash}/bin/bash … with no quoting, and checks/static-site.nix:85 expects that same spelling. types.pathInStore accepts a store path containing a space, so a caller passing "${site}/public files" produces a command that word-splits: the system build tries to execute files. Quote the interpolation and fix the expectation with it.

2. The reference scan fails open

checks/site-content.sh:86 ends its pipeline with | grep '^/' || true. The || true is there because grep legitimately exits non-zero when a site has no root-relative references, but it also swallows a real failure anywhere in the pipeline: if sed is missing or grep hits a read error, the reference set comes back empty and the check passes having scanned nothing. Distinguish "no matches" from "the scan broke".

3. The closed-port assertion cannot fail for the right reason

checks/static-site.nix:175 asserts pages.fail("ss -ltnH 'sport = :22' | grep -q ."). A malformed filter expression makes ss error, grep finds nothing, and the subtest passes while claiming port 22 is closed. The command -v ss guard on the line above shows the missing-binary case was considered, but it does not cover a filter that parses wrongly. Assert on ss succeeding and producing no matching line, rather than on the pipeline failing.

4. The context assertion does not check whose context

modules/static-site.nix:29 requires builtins.getContext siteRoot != { }, which any non-empty context satisfies. Context belonging to an unrelated derivation passes while the site itself stays outside the system closure and can be garbage-collected. Check that the context names the path being served.

5. Two gaps in the reference scanner

checks/site-content.sh:84 is line-oriented, so an attribute whose whitespace around = crosses a newline is not seen: <img src= followed by "/missing.png"> on the next line passes while the browser gets a 404. Separately, :86 treats a leading slash as proof of a local path, so a protocol-relative URL such as //cdn.example/app.js is looked for inside the site root and reported missing — a valid page failing the build. Both were found independently by both passes.

6. The title assertion accepts a comment

checks/site-content.sh:25 greps for a substring, so an index containing only  satisfies it with no title element present.

Why these are grouped

Four of the six are the same failure: a check that passes without establishing what it claims. That pattern has now been confirmed repeatedly in this material — a fabricated SSH key behind a grep, a host-key comparison matching a key blob against a key-type string, a serving assertion naming a title that existed only in a placeholder, and now these. The fixture suites are worth extending case by case rather than rewriting, since each case here is a specific input nobody thought to plant.

What both passes checked and found sound

Worth recording so it is not re-reviewed: both kept assertions in modules/public-host.nix fire for the right reason; the firewall expectations hold only because allowedTCPPorts sorts and uniques, which was confirmed in the pinned nixpkgs rather than assumed; the VM test's SSH pair is genuinely matched with a genuinely unrelated negative key; system.checks really is a dependency of the system build; the sshd fail2ban jail really is keyed on services.openssh.ports; and types.pathInStore does reject lexical ../.. traversal, so that assertion is not a tautology. All three shell checks are shellcheck-clean.

Close six defects found by two independent adversarial review passes over the merged static-site files. The symlink-root escape those passes also found is filed separately as #61, which is the one with a security consequence; these are the rest. Both passes reproduced their claims against source before reporting. Each is verified. ### 1. The site root reaches a shell unquoted `modules/static-site.nix:7` writes `SITE_ROOT=${siteRoot} ${pkgs.bash}/bin/bash …` with no quoting, and `checks/static-site.nix:85` expects that same spelling. `types.pathInStore` accepts a store path containing a space, so a caller passing `"${site}/public files"` produces a command that word-splits: the system build tries to execute `files`. Quote the interpolation and fix the expectation with it. ### 2. The reference scan fails open `checks/site-content.sh:86` ends its pipeline with `| grep '^/' || true`. The `|| true` is there because `grep` legitimately exits non-zero when a site has no root-relative references, but it also swallows a real failure anywhere in the pipeline: if `sed` is missing or `grep` hits a read error, the reference set comes back empty and the check passes having scanned nothing. Distinguish "no matches" from "the scan broke". ### 3. The closed-port assertion cannot fail for the right reason `checks/static-site.nix:175` asserts `pages.fail("ss -ltnH 'sport = :22' | grep -q .")`. A malformed filter expression makes `ss` error, `grep` finds nothing, and the subtest passes while claiming port 22 is closed. The `command -v ss` guard on the line above shows the missing-binary case was considered, but it does not cover a filter that parses wrongly. Assert on `ss` succeeding and producing no matching line, rather than on the pipeline failing. ### 4. The context assertion does not check whose context `modules/static-site.nix:29` requires `builtins.getContext siteRoot != { }`, which any non-empty context satisfies. Context belonging to an unrelated derivation passes while the site itself stays outside the system closure and can be garbage-collected. Check that the context names the path being served. ### 5. Two gaps in the reference scanner `checks/site-content.sh:84` is line-oriented, so an attribute whose whitespace around `=` crosses a newline is not seen: `<img src=` followed by `"/missing.png">` on the next line passes while the browser gets a 404. Separately, `:86` treats a leading slash as proof of a local path, so a protocol-relative URL such as `//cdn.example/app.js` is looked for inside the site root and reported missing — a valid page failing the build. Both were found independently by both passes. ### 6. The title assertion accepts a comment `checks/site-content.sh:25` greps for a substring, so an index containing only `` satisfies it with no title element present. ### Why these are grouped Four of the six are the same failure: a check that passes without establishing what it claims. That pattern has now been confirmed repeatedly in this material — a fabricated SSH key behind a grep, a host-key comparison matching a key blob against a key-type string, a serving assertion naming a title that existed only in a placeholder, and now these. The fixture suites are worth extending case by case rather than rewriting, since each case here is a specific input nobody thought to plant. ### What both passes checked and found sound Worth recording so it is not re-reviewed: both kept assertions in `modules/public-host.nix` fire for the right reason; the firewall expectations hold only because `allowedTCPPorts` sorts and uniques, which was confirmed in the pinned nixpkgs rather than assumed; the VM test's SSH pair is genuinely matched with a genuinely unrelated negative key; `system.checks` really is a dependency of the system build; the `sshd` fail2ban jail really is keyed on `services.openssh.ports`; and `types.pathInStore` does reject lexical `../..` traversal, so that assertion is not a tautology. All three shell checks are shellcheck-clean.