VayuPress Docs

ADR-0157 — Nothing may write into a directory nginx includes

  • Status: Accepted; shipped in v3.17.14
  • Date: 2026-08-06
  • Follows ADR-0155 (certificates without a restart) and ADR-0156 (the write connection). Both were real defects. Neither was this one.

0. Three wrong answers first

An operator reported, four times over two days, that adding a domain and provisioning its certificate took the site to 502 for minutes, after which it recovered on its own. Three fixes shipped against that report:

Release What was fixed Was it the cause?
v3.17.11 three unnecessary nginx/app restarts removed no
v3.17.12 helpers opened the DB with the server's full init no
v3.17.13 traffic queued on the single write connection no

Every one of those was a genuine defect, found by reading code and proved by a test. Not one of them was diagnosed from the failure. The pattern is the finding: a mechanism that plausibly produces the reported symptom is not evidence that it produced this symptom, and shipping against a plausible mechanism three times in a row is how two days go by with the fault untouched.

What ended it was one probe against the running install during an outage.

1. The evidence, in the order it landed

The app was never the problem. During a 502, on the box:

$ curl -v -m 8 http://127.0.0.1:8080/health
{"status":"alive","version":"3.17.13","uptime_seconds":884.05}

That endpoint touches no database. It answered in 1 ms. The journal showed continuous service throughout — 200s, 304s and 404s at 1–35 ms. Nothing was blocked, stalled or queued. Every theory in the table above was excluded by a single request.

nginx never generated a 502 either. grep ' 502 ' access.log returned nothing at all. Whatever the browser was rendering, nginx did not produce it.

What the error log did show, clustered at the exact minute the provisioning run wrote its vhosts:

09:11:13 [alert] *9550 open socket #17 left in connection 10
09:11:13 [alert] aborting
09:11:17 [alert] aborting
09:11:20 [alert] aborting
09:11:31 [alert] aborting

And, repeated on every reload for three days:

[warn] conflicting server name "mcp.johal.in" on 0.0.0.0:443, ignored

Then the directory listing that explained it:

lrwxrwxrwx  vayupress-mcp -> /etc/nginx/sites-available/vayupress-mcp
-rw-r--r--  vayupress-mcp.vayushield.bak      <-- a REGULAR FILE, dated 3 Aug

Every other entry was a symlink.

2. Defect A — a backup was live configuration

deploy/vayushield-agent.sh globs /etc/nginx/sites-enabled/* to find the MCP vhost, then saved its pre-change copy beside it:

found=…                                   # from a glob over sites-enabled
local bak="${found}.vayushield.bak"
cp -f "$found" "$bak"

nginx includes that directory with include /etc/nginx/sites-enabled/*; — a bare glob, no extension filter, no exclusions. The backup was not a backup. It was a second live server block from the moment it was written, and it had been parsed on every reload for three days.

nginx resolves two blocks claiming one name by keeping whichever the glob reached first and discarding the other, and says so once, at warn level. Which one loses depends on filename ordering — which is to say, on nothing an operator would ever think to check.

The other two backup sites in that script write into conf.d/, which is included as *.conf; a .conf.vayushield.bak never matched, so those were inert. Only the sites-enabled one was live — and it is the only one whose path came from a glob over an include directory, which is exactly the coupling that made it dangerous.

3. Defect B — the reload storm

scripts/setup-vayudomain.sh reloads nginx twice per host: once for the HTTP-only vhost that lets certbot validate, once for the real vhost after the certificate exists. Six domains is twelve reloads, and the mcp/api/talk/ openpgpkey helpers add their own — sixteen in about ninety seconds on the install that reported this.

Each reload starts a worker generation and asks the previous one to drain. Issue the next before that finishes and generations pile up, until nginx stops waiting: open socket … left in connection then aborting is a worker being retired while it still holds live connections. Every request in flight on it dies mid-response. The client sees a dropped connection, and anything in front of nginx renders that as 502 — which is why nginx's own access log recorded none.

Nothing here is nginx misbehaving. It is being asked to reload faster than it can retire a generation, which is a thing only the caller can fix.

4. What was built

A backup never lands where configuration is read from. shield_backup writes to a dedicated directory outside every include path. Existing installs are repaired rather than only new ones: sweep_stray_nginx_backups runs on every reconcile and moves .bak, .save, .orig, .dpkg-old, .dpkg-dist and ~ files out of sites-enabled. Only regular files, and only those suffixes — a symlink is how a vhost is enabled, and a plainly-named regular file may be an operator's hand-written vhost. Sweeping either would take a site down to fix a tidiness problem.

Reloads no longer overlap. await_nginx_drain waits for the previous generation before another reload is issued. The wait is observed rather than guessed — nginx renames a draining worker's process title to worker process is shutting down — so on the common case of a single-domain install with nothing draining it costs nothing at all. A genuinely long-lived connection is capped, and proceeding past the cap says so out loud, because that is the one remaining path that can still drop a request.

The panel names the collision. The Sites page reports any hostname declared by more than one file, naming both files, and any backup sitting in the include path. It renders nothing on a healthy install — this exists to surface a fault that lived in a warn-level log line, and a permanent green panel for a condition almost nobody has is the same noise that made the original warning invisible.

Two things the checker deliberately does not do: it does not parse nginx's grammar or resolve includes, and it does not predict which block wins. That depends on glob order and listen addresses, and a panel that guessed would be worse than one that stays quiet.

5. What the audit found — in this change's own code

A local privilege-escalation primitive, introduced by the fix. The first version put the backup directory at /var/lib/vayupress/nginx-backups. That tree is owned by the unprivileged service user; the agent doing the moving runs as root. So the service user could pre-create nginx-backups as a symlink and root's own mv would follow it, depositing files into any directory they chose. The fix for an outage would have shipped an escalation alongside it.

It moved to /var/backups, which is root-owned on every Debian-family install — removing the primitive rather than guarding it — and a symlinked destination is refused outright as defence in depth.

The first mutation written for that guard survived, and the mutation was wrong rather than the test: it removed one of two redundant symlink checks and the second still caught the plant. Re-run with both removed, the test failed exactly as it should, naming the file root had been made to write.

An unbounded read on a page view. The checker reads every file in the directory — including through symlinks, as nginx does — on every Sites page load. Capped at 512 KB per file. A generated vhost here is under 4 KB.

Attacked and found sound: the sweep's suffix list against a hand-written vhost and against a symlink (both correctly left alone); the duplicate detection against a normal vhost, which declares its hostname twice, once per listen block, and must not be flagged; the catch-all server_name _, which every install shares by design; and the card's escaping, against a hostile filename and a hostile server_name.

6. How it was proven

Nine mutations across the shell and Go changes, all killed. The shell functions are executed against fixture directories rather than read for strings — a source-level check would pass on a script that contains the right words and does the wrong thing, which is how a backup came to live in an include path in the first place. await_nginx_drain is exercised against a stubbed ps that reports a draining worker three times and then a clean table.

One test-quality note, because it repeats a lesson from ADR-0156: the duplicate check initially anchored server_name to the start of a line. That silently missed server { listen 80; server_name x; } written on one line — valid nginx, and the format of this product's own generated vhosts. A check that only saw tidily-formatted configuration would have reported the incident that prompted it as clean.

7. The rule this ADR is named for

Nothing may write a file into a directory another program reads with a glob. Not a backup, not a temp file, not a .orig. The reader's include pattern is not part of the writer's interface and may change without notice; a file's safety must never depend on the exact shape of somebody else's glob.

And the process lesson, which cost more than the bug: a plausible mechanism is not a diagnosis. Three correct fixes shipped against a fault none of them touched. The probe that settled it — one request to an endpoint that needs no database, taken while the site was actually failing — was available on the first report and should have been the first thing asked for.