fix: harden the values a project can put into generated files, and close the gaps around them - #1204
Merged
Conversation
A project's committed .lerd.yaml supplies values that lerd interpolates into files it generates, and three of those paths passed them through unexamined. A node_version reached the command line of a host worker unit, where fnm took it unquoted and nvm interpolated it into a single-quoted message; a custom_workers entry's label, restart and schedule each reached a line of the unit, where only command was checked; and a domains entry reached the vhost's server_name, where a semicolon and a pair of braces close the server block and open another. Each one let a checkout decide what ran on the machine that linked it. Values are now checked where the file is generated rather than field by field. Node versions must be version shaped before they reach a manager, and both managers quote what they pass. Every value spliced into a worker unit goes through one guard, so a field added later is covered without anyone remembering. Every vhost renders through one helper that rejects the characters that end a directive, and a domain carrying them is refused when the site is registered so the error arrives at link time. Paths are checked only for a line break, since they reach the template quoted and a directory legitimately named my#project;v2 still has to serve. The database name in the existence check is escaped for the literal it lands in, which the helpers deleted during the entity refactor used to cover.
… the phar The pinned tool manifest is fetched from a branch before every download, so it reaches every install without going through a release. A pin was accepted from any host as long as the URL began with https, which made one file enough to change what every machine downloads and marks executable. A published pin is now honoured only when it points at a host lerd fetches tools from, extendable through LERD_TOOLS_HOSTS, and may carry a sha256 per asset that the download is checked against. Bytes land in a temporary file and are moved into place only once complete, so a rejected or truncated download cannot replace a working binary. Composer's version was only ever knowable from the stamp sidecar, and that is written only when lerd downloads it, which it does only when the file is absent. Any install that already had composer.phar read as unknown forever and, because the update check needs a known version, was never offered an update either. The version is compiled into the phar, so it is read from the bytes rather than by running it, and stamped on the way out so the read happens once. One tool that cannot be updated no longer decides the fate of the others: each is an independent download, so the rest are attempted and the failure is reported at the end.
lerd bootstrap --system writes /etc/sudoers.d/lerd for whichever user it resolved, and that name became the first field of every rule in the file with nothing checking it. A name carrying a space produces a drop-in sudo rejects outright, which loses the DNS grant it was meant to install. The name now has to look like a user name, and the rendered file is put through visudo before it is installed. A host without visudo still gets its grant, since the rule shape is already constrained by then.
The base-image freshness warm-up invalidated the digest cache before reading it, so a registry that could not answer was asked again on every status rebuild instead of once per cache window. It now reads through the cache, and only the manual check bypasses it. A database import built its own container invocation and ignored the client image the engine's entity declares, which export already honoured, so an engine whose own image ships no tooling would export and then fail to import. A pinned dependency host was rewritten as a bare substring, so an environment value naming a versioned install of the same engine came out pointing at a host that does not exist. The rewrite now stops at a name boundary. The JetBrains sync wrote both files even when it had decided to leave the user's own connection alone, which created an empty credentials sidecar in a project it had just chosen not to touch and moved the other file's mtime for nothing. The stop cascade gained a visited set. Dependents are matched by family and env_role rather than by name alone, so a service whose own family satisfies one of its depends_on entries is its own dependent, and the cascade did not return. Also: an owner is resolved from a project's env values in a fixed order rather than whichever the map yielded first, the tunnel builder answers with an error instead of a nil command for a mode it does not know, and the worker-failure detector seam is read under a lock because the flush runs on a timer.
…manager Three capabilities had no tool at all. Postgres extensions were reachable only from the CLI, so an assistant could not see what an engine offers or add one before a dump arrives. Everything a preset declares that is not a database, the buckets a RustFS install holds today, was reachable only from the dashboard. And the reference described the fnm and nvm split while offering nothing to act on it. The reference also now says what is deliberately absent, so an assistant stops looking: host tools are reported but applying an update is a CLI command, and sharing a site is not exposed at all, since putting a site on the public internet is the user's call to make. Export and import on an entity stream a file, so they stay on the CLI and the dashboard and the handler says so rather than half working. The manifest ceiling moves for the added actions, with description verbosity in the same three tools trimmed to pay for part of it.
…shes The README's feature list had grown to where single entries ran past a dozen rendered lines, one of them carrying six separate features in a single bullet. Each entry is now three or four lines. What went with it is worth knowing: the services entry lost several engines from its list along with db:move, the client shims and the port auto-shift, so those are candidates for entries of their own if you want them visible. The list also described how lerd is built rather than what it does, so the framework-agnostic and not-hardcoded framing is gone. That is true of the code and interesting to whoever works on it, and it tells a user nothing. Documented alongside the code they describe: the version shape every Node source has to have, the worker fields that may not carry a newline, the hostname characters a domains entry may not use, where the tool pins come from and what a digests entry does, and that one tool failing to update no longer stops the rest. Corrected the claim that a JetBrains connection carries its credentials in the JDBC URL, which it does not and never did. Em dashes are gone from the prose. Left in place where a page reproduces what lerd prints, since those quote real output, and left in the changelog, which is a record of what shipped.
…nning Definitions exist for the recent majors only, so an older project borrows the nearest and is marked guessed. The version switch clamped to that borrowed range anyway, which refuses the PHP the project actually requires: on a Laravel 8 app, `lerd isolate 7.4` answered "7.4 isn't usable here" and moved it to 8.5. Linking already declined to clamp a guessed definition, so the two disagreed about the same project. A definition now governs only when it is the real one for the detected version. Otherwise, and when no framework was recognised at all, the constraint comes from the project's own composer.json, which is what it says it supports. That is also better than the previous no-clamp-when-guessed behaviour, since a project requiring 8.2 can no longer be handed a PHP it cannot run. Alternation is matched on either spelling. Composer accepts a single pipe and the projects this matters for use it, Laravel 8 requiring "^7.3|^8.0", so the whole constraint was read as one term that nothing satisfied. Detection then fell through to the constraint's minimum and pinned 7.3 on a host running 8.5.
The guard script macOS writes embeds the version manager's prefix as a line of a shell script, so it carries the same quoting the Linux unit does and its assertion had to move with it. The clean-write assertion read a systemd path, which does not exist on macOS, and the refusal check looked for systemd unit filenames, so it passed there without reading anything. The refusal now walks the tree, which is true on both platforms, and what a written unit says is asserted where systemd is the target.
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.
Closes #1203
Four review passes over the v1.30.1 to v1.31.0-beta.1 window. Each finding was reproduced first, fixed, then re-measured on the ubuntu26.04 and omarchy VMs against the same reproduction.
The substance is that a checked-out project could decide what runs on the machine that links it. A
node_versionreached a host worker unit's command line, unquoted through fnm and inside a single-quoted message through nvm. Acustom_workersentry's label, restart and schedule each reached a line of that unit, where only command was ever checked, and systemd ran anExecStartPreone of them injected. Adomainsentry reached the vhost'sserver_name, where a semicolon and two braces close the server block and open another, andnginx -taccepted the result, so a project served its own block on a domain of its choosing. All three predate this window rather than arriving in it.The fixes check values where the file is generated rather than field by field, so a field added later is covered without anyone remembering. Paths are treated differently from names, because they reach the template quoted and a directory legitimately called
my#project;v2still has to serve.Around that: published tool pins are constrained to the hosts lerd fetches tools from and may carry a checksum, since that manifest reaches every install without a release; composer's version is read out of the phar rather than guessed, which fixes a card that showed a question mark on every install that had composer before stamping existed, and restores the update flag that silently required a known version; a sudoers drop-in is validated before it lands; and a handful of correctness issues in the service, database and IDE paths, including a base-image check that asked an unreachable registry again on every status rebuild.
MCP gained the three surfaces it was missing and now states what is deliberately absent, so an assistant stops looking for a way to share a site.
The README feature list is down to three or four lines an entry, and the framing about how lerd is built rather than what it does is gone from it.
Two things left deliberately. Sharing a site is still not exposed over MCP. And the em dashes in the strings lerd itself prints are untouched, since that is program output rather than docs and belongs in its own change.