57/T1: A file resource may name a template file in its module's repository (issue 654) #313

Merged
mesh-admin merged 5 commits from pkg/57-t1-template-files into feat/57-config-and-provisioning 2026-10-14 01:00:48 +00:00
Contributor

Work package T1 of hq design 57 (ADR 0349 clause 2), issue 654.

What changed

  • Manifest (internal/catalogue/template.go, manifest.go): a file resource takes "template": "files/<name>" in place of content. ParseManifest refuses, by resource id: a file naming both template and content; a template on a resource that is not a file; a template path that is not a clean relative path under files/ (../x, /etc/x, files, files/../x, files/./x, other/x, backslashes); a module naming templates whose build declares no archive artifact built from: "files" (or two of them).
  • The templates archive is an ordinary archive artifact the module declares, built from: "files" with the builder's existing packing (pack, PublishArchive): nothing new is built. I chose to have the module declare it rather than have the builder add one silently, because design 57's T5 says modules without a build "gain an archive build for their templates", and a declared artifact is what the source fingerprint, reuse key and planner already understand.
  • Builder (internal/builder/template.go, builder.go): after the artifacts are made or taken, withTemplateTexts packs files/ again, checks the bytes hash to the templates archive's recorded digest, and reads each file's text back from those packed bytes into Built.Texts (json:"-": never carried or kept). Manifest.Resolve reads each template's text from there into content and drops template; a missing template or non-UTF-8 text is refused by name. A missing template is also refused before anything is built (the build judges the manifest with its templates read from the tree).
  • module check (check.go), merge gate (merge_gate.go shelfWithChange, ahead of standInBuild), module add (modules.go) read templates from the repository tree beside the manifest (catalogue.WithTemplatesFrom), then read the manifest again strictly, so each rule about a file's text judges a template's text exactly as inline content.
  • Registration (inventory.RegisterModule) refuses a manifest that still names a template (HeldTemplateProblem): one built by a builder that predates templates would otherwise compose files with no text.

Where the text is read from at registration, and why

The text is read by the builder, from the packed archive, on the build node, and the controller registers the resolved manifest as the build result carries it. The controller does not fetch the archive by digest from the artifact store. Why:

  1. The text is bound to the digest anyway: the builder reads it from the very bytes it hashed (a fresh build), or from the tree packed again and checked to hash to the digest a walk took from its check's build (ADR 0350). A tree that packs to another digest is refused.
  2. The controller already trusts the build result for every digest it registers, and never fetches artifacts today. A fetch at registration would add a new failure path (store unreachable) to every registration, plus a client the controller does not have.
  3. The manifest the mesh holds stays self-contained (content inline, no template key). So composition, the node-engine, the merge gate's snapshot and an older controller reading the store after a rollback are all unchanged.
    Later packages (the provisioner) can make the declaration name the archive instead (ADR 0349 clause 2's end state). That needs this archive's digest, which the build already records.

Order with older readers (the unknown-field rule)

Resources are free-form maps in the controller, so an older controller does not refuse template; it would pass it through to the node-engine, whose strict parse refuses it. The registration guard covers an older builder with a newer controller. No manifest in the catalogue uses template in this package. The first that does (T2, issue 655) must wait until this controller and the builder built from it run on every build node.

How it is proven (tests; each failed before the change)

  • Byte-identical composition: internal/catalogue TestATemplatedModuleComposesByteIdenticallyToItsInlineTwin (archive path) and TestATemplatedModuleReadFromItsTreeComposesByteIdenticallyToItsInlineTwin (tree path); cmd TestTheMergeGateResolvesATemplatedModuleToItsInlineTwin.
  • The merge gate judges a change to a template alone: cmd TestTheMergeGateJudgesAChangeToATemplateAlone. Moving the module to its template with the same text changes no module. Changing only files/notifier.conf marks notifier changed and rebuilds only notifier. Text the controller refuses fails the pull request, naming the module. A removed template fails, naming it.
  • module check: TestModuleCheckReadsTemplatesFromTheTreeBesideTheManifest (ok, refused text, missing template).
  • Refusals: TestAFileNamingBothATemplateAndContentIsRefused, TestATemplateOutsideFilesIsRefusedByName, TestATemplateOnAResourceThatIsNotAFileIsRefused, TestATemplatedModuleWhoseBuildPacksNoFilesIsRefused, TestAMissingTemplateIsRefusedByName, TestATemplateThatIsASymlinkIsRefused, TestAResolveWithoutTheArchivesTextIsRefused, TestATemplateThatIsNotUTF8IsRefused, TestAManifestStillNamingATemplateIsNotHeld; internal/inventory TestAManifestStillNamingATemplateIsNotRegistered.
  • Builder: TestABuildReadsEachTemplateBackFromItsArchive, TestABuildWhoseTemplateIsMissingFailsNamingIt, TestAWalkTakingAChecksBuildReadsTheTemplatesBackToo (reuse path, nothing built), TestTemplateTextsAreRefusedWhenTheTreeDoesNotPackToTheDigestTaken.
  • Existing tests: unchanged, except two helpers in internal/builder/reuse_test.go. They are split (aModuleWith, aCheckBuiltBy) so the reuse test can use a templated repository; the existing callers behave as before.

Left for other packages

  • Moving modules to templates: T2 to T6 (issues 655 to 659), in mesh-catalog and the applications.
  • The declaration naming the archive instead of the text, and the provisioner filling templates: the provisioner packages (P*).
  • Per-artifact fingerprinting, so a template change rebuilds only its archive: issue 607.

Reviewer's checklist

  1. Does what issue 654 "Expected" and design 57 T1 say: template field, files/ packed by the existing archive packing, Resolve and registration read text back, module check and the merge gate read the tree, both-forms refused. Nothing else changes.
  2. Done-when proof exists as tests that failed before. I checked by reverting check.go and merge_gate.go: the module check and gate tests fail. The catalogue tests failed before Resolve and ParseManifest were wired. Existing tests are untouched apart from the helper split above.
  3. Byte-identical composition tests for the templated form against the inline form (archive and tree paths). A module that has not moved composes as before: no manifest path changes for a manifest without template, and TestTheMeshAsItIsPassesTheGate plus the full suite pass.
  4. No secret in logs, events, fixtures or commits. No database change and no migration (none needed).
  5. The new key is accepted by the controller and the builder before any manifest writes it. None in the catalogue does, and the node-engine never sees it (Resolve drops it, registration refuses a manifest still carrying it).
  6. gofmt -l cmd internal examples is empty. CGO_ENABLED=0 go vet ./... is clean. sh merge-check.sh, with MESH_TEST_POSTGRES pointing at a throwaway PostgreSQL, exited 0: the whole suite under -race, plus the store tests it lists.
  7. Words: template, controller, node-engine, walk, build. No retired words. Internal names carry what they are.

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

Work package T1 of hq design 57 (ADR 0349 clause 2), issue 654. ## What changed - **Manifest (`internal/catalogue/template.go`, `manifest.go`)**: a file resource takes `"template": "files/<name>"` in place of `content`. `ParseManifest` refuses, by resource id: a file naming both `template` and `content`; a template on a resource that is not a file; a template path that is not a clean relative path under `files/` (`../x`, `/etc/x`, `files`, `files/../x`, `files/./x`, `other/x`, backslashes); a module naming templates whose build declares no archive artifact built `from: "files"` (or two of them). - **The templates archive** is an ordinary `archive` artifact the module declares, built `from: "files"` with the builder's existing packing (`pack`, `PublishArchive`): nothing new is built. I chose to have the module declare it rather than have the builder add one silently, because design 57's T5 says modules without a build "gain an archive build for their templates", and a declared artifact is what the source fingerprint, reuse key and planner already understand. - **Builder (`internal/builder/template.go`, `builder.go`)**: after the artifacts are made or taken, `withTemplateTexts` packs `files/` again, checks the bytes hash to the templates archive's recorded digest, and reads each file's text back from those packed bytes into `Built.Texts` (`json:"-"`: never carried or kept). `Manifest.Resolve` reads each template's text from there into `content` and drops `template`; a missing template or non-UTF-8 text is refused by name. A missing template is also refused before anything is built (the build judges the manifest with its templates read from the tree). - **module check (`check.go`), merge gate (`merge_gate.go` shelfWithChange, ahead of standInBuild), `module add` (`modules.go`)** read templates from the repository tree beside the manifest (`catalogue.WithTemplatesFrom`), then read the manifest again strictly, so each rule about a file's text judges a template's text exactly as inline content. - **Registration (`inventory.RegisterModule`)** refuses a manifest that still names a template (`HeldTemplateProblem`): one built by a builder that predates templates would otherwise compose files with no text. ## Where the text is read from at registration, and why The text is read by the **builder, from the packed archive, on the build node**, and the controller registers the resolved manifest as the build result carries it. The controller does **not** fetch the archive by digest from the artifact store. Why: 1. The text is bound to the digest anyway: the builder reads it from the very bytes it hashed (a fresh build), or from the tree packed again and checked to hash to the digest a walk took from its check's build (ADR 0350). A tree that packs to another digest is refused. 2. The controller already trusts the build result for every digest it registers, and never fetches artifacts today. A fetch at registration would add a new failure path (store unreachable) to every registration, plus a client the controller does not have. 3. The manifest the mesh holds stays self-contained (`content` inline, no `template` key). So composition, the node-engine, the merge gate's snapshot and an older controller reading the store after a rollback are all unchanged. Later packages (the provisioner) can make the declaration name the archive instead (ADR 0349 clause 2's end state). That needs this archive's digest, which the build already records. ## Order with older readers (the unknown-field rule) Resources are free-form maps in the controller, so an older controller does not refuse `template`; it would pass it through to the node-engine, whose strict parse refuses it. The registration guard covers an older **builder** with a newer controller. **No manifest in the catalogue uses `template` in this package.** The first that does (T2, issue 655) must wait until this controller and the builder built from it run on every build node. ## How it is proven (tests; each failed before the change) - Byte-identical composition: `internal/catalogue` `TestATemplatedModuleComposesByteIdenticallyToItsInlineTwin` (archive path) and `TestATemplatedModuleReadFromItsTreeComposesByteIdenticallyToItsInlineTwin` (tree path); `cmd` `TestTheMergeGateResolvesATemplatedModuleToItsInlineTwin`. - The merge gate judges a change to a template alone: `cmd` `TestTheMergeGateJudgesAChangeToATemplateAlone`. Moving the module to its template with the same text changes no module. Changing only `files/notifier.conf` marks notifier changed and rebuilds only notifier. Text the controller refuses fails the pull request, naming the module. A removed template fails, naming it. - module check: `TestModuleCheckReadsTemplatesFromTheTreeBesideTheManifest` (ok, refused text, missing template). - Refusals: `TestAFileNamingBothATemplateAndContentIsRefused`, `TestATemplateOutsideFilesIsRefusedByName`, `TestATemplateOnAResourceThatIsNotAFileIsRefused`, `TestATemplatedModuleWhoseBuildPacksNoFilesIsRefused`, `TestAMissingTemplateIsRefusedByName`, `TestATemplateThatIsASymlinkIsRefused`, `TestAResolveWithoutTheArchivesTextIsRefused`, `TestATemplateThatIsNotUTF8IsRefused`, `TestAManifestStillNamingATemplateIsNotHeld`; `internal/inventory` `TestAManifestStillNamingATemplateIsNotRegistered`. - Builder: `TestABuildReadsEachTemplateBackFromItsArchive`, `TestABuildWhoseTemplateIsMissingFailsNamingIt`, `TestAWalkTakingAChecksBuildReadsTheTemplatesBackToo` (reuse path, nothing built), `TestTemplateTextsAreRefusedWhenTheTreeDoesNotPackToTheDigestTaken`. - Existing tests: unchanged, except two helpers in `internal/builder/reuse_test.go`. They are split (`aModuleWith`, `aCheckBuiltBy`) so the reuse test can use a templated repository; the existing callers behave as before. ## Left for other packages - Moving modules to templates: T2 to T6 (issues 655 to 659), in mesh-catalog and the applications. - The declaration naming the archive instead of the text, and the provisioner filling templates: the provisioner packages (P*). - Per-artifact fingerprinting, so a template change rebuilds only its archive: issue 607. ## Reviewer's checklist 1. [x] Does what issue 654 "Expected" and design 57 T1 say: template field, files/ packed by the existing archive packing, Resolve and registration read text back, module check and the merge gate read the tree, both-forms refused. Nothing else changes. 2. [x] Done-when proof exists as tests that failed before. I checked by reverting check.go and merge_gate.go: the module check and gate tests fail. The catalogue tests failed before Resolve and ParseManifest were wired. Existing tests are untouched apart from the helper split above. 3. [x] Byte-identical composition tests for the templated form against the inline form (archive and tree paths). A module that has not moved composes as before: no manifest path changes for a manifest without `template`, and `TestTheMeshAsItIsPassesTheGate` plus the full suite pass. 4. [x] No secret in logs, events, fixtures or commits. No database change and no migration (none needed). 5. [x] The new key is accepted by the controller and the builder before any manifest writes it. None in the catalogue does, and the node-engine never sees it (Resolve drops it, registration refuses a manifest still carrying it). 6. [x] `gofmt -l cmd internal examples` is empty. `CGO_ENABLED=0 go vet ./...` is clean. `sh merge-check.sh`, with `MESH_TEST_POSTGRES` pointing at a throwaway PostgreSQL, exited 0: the whole suite under `-race`, plus the store tests it lists. 7. [x] Words: template, controller, node-engine, walk, build. No retired words. Internal names carry what they are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mesh-admin added 4 commits 2026-10-13 23:59:56 +00:00
A file's text may live in files/<name> beside the manifest instead of inline,
so modules can move their files out of module.json (ADR 0349 clause 2, issue
654). The build packs files/ into the module's declared archive and reads each
template back from the packed bytes, proven by the digest, into content: the
manifest the mesh holds, composition and the node-engine stay as they are.
module check, the merge gate and module add read templates from the tree beside
the manifest, so a change to a template alone is judged. A file naming both
template and content, a template outside files/, a missing one, and a held
manifest that still names one are refused by name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Let a file resource name a template file in its module's repository: composition, check and gate tests
mesh/merge-gate head f45e0a1de508 merged into trunk bfc79c64d989 — pass: the change touches no module of the mesh's graph
mesh/repo-check head f45e0a1de508 merged into trunk bfc79c64d989 — pass: its merge-check.sh passed
mesh/delivery superseded: a newer head of the same pull request
f45e0a1de5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Contributor

Delivery novox/mesh-controller@def3ccefe3c8 — delivered since 2026-10-14T01:01:11Z

Delivery plan — builds nothing: the change touches no module of the mesh's graph

Transitions

  • 2026-10-14 00:52 (new) → proposed (announced): novox/mesh-controller#313's head announced
  • 2026-10-14 00:56 proposed → checked (checked): a verdict names this commit and the trunk's head as its trunk commit
  • 2026-10-14 00:56 checked → ready (accepted): the gate passed or warned, and the repository's own check did not fail
  • 2026-10-14 01:01 ready → published (merged): on the trunk its modules follow: merged there, and its walk opened — or nothing for a walk to move
  • 2026-10-14 01:01 published → delivered (done): nothing for a walk to move

The commit's note under refs/notes/mesh-plan keeps every transition: git log --notes=mesh-plan.

<!-- mesh-delivery:view --> **Delivery** `novox/mesh-controller@def3ccefe3c8` — **delivered** since 2026-10-14T01:01:11Z **Delivery plan** — builds nothing: the change touches no module of the mesh's graph **Transitions** - 2026-10-14 00:52 (new) → proposed (announced): novox/mesh-controller#313's head announced - 2026-10-14 00:56 proposed → checked (checked): a verdict names this commit and the trunk's head as its trunk commit - 2026-10-14 00:56 checked → ready (accepted): the gate passed or warned, and the repository's own check did not fail - 2026-10-14 01:01 ready → published (merged): on the trunk its modules follow: merged there, and its walk opened — or nothing for a walk to move - 2026-10-14 01:01 published → delivered (done): nothing for a walk to move The commit's note under `refs/notes/mesh-plan` keeps every transition: `git log --notes=mesh-plan`.
Author
Contributor

Verdict: CHANGES at f45e0a1de5

Checks I ran in my own worktree of this head: sh merge-check.sh with MESH_TEST_POSTGRES pointing at a throwaway PostgreSQL over TCP exited 0. That covers gofmt -l (empty), CGO_ENABLED=0 go vet ./... (clean), the whole suite under -race, and every listed store test passing. I also reverted the wiring locally (the builder's withTemplateTexts call, the gate's and module check's WithTemplatesFrom, and the registration guard). The builder, module check, merge gate ("judges a change to a template alone") and registration tests then fail, as they should.

The points I was asked to judge:

  • (a) Byte-identical composition holds. A manifest without template takes no new path: Templated() is false, so WithTemplatesFrom returns it unchanged and Resolve skips the new block. The inline twin and the templated twin compose identically on both the archive path and the tree path.
  • (b) The reuse path is sound. withTemplateTexts packs files/ again with the same pack (internal/builder/builder.go:1238) and reads text only from bytes whose sha256 equals built[at].Digest. On a walk that takes a check's build, that is the digest it took, so the text cannot differ from the archive the mesh records. A tree that packs differently fails the build loudly.
  • (d) The author's statement is true. I checked it on the base branch: a manifest with template on a file parses, resolves and declares with "template" passed through to the node. mesh-host's strictDecode (DisallowUnknownFields) has no such field and refuses it. An older controller's merge gate would also pass such a manifest, so nothing automatic enforces the order. No catalogue manifest may use template until this controller and the builder built from it run on every build node, that is, after the release branch reaches main and its walk is done. Issue 655 (T2) should say so.
  • (e) Confirmed: the gate fills the templates (merge_gate.go:582) before standInBuild, and the filled manifest no longer names a template.

Findings:

  1. (must) internal/catalogue/template.go:200-211. ReadTemplates checks only the last path component with Lstat, so a symlinked files/ or files/<dir> is followed out of the tree. I proved it: with files -> /some/dir and with files/sub -> /some/dir, WithTemplatesFrom returned the outside file's text with no error. This is the reader the merge gate (merge_gate.go:582) runs on a pull request's tree on a build node, and module check and module add use it too. It contradicts the package's own rule that symlinks are refused, and pack refuses them, so the gate can pass a tree the build will refuse. Fix: read through os.OpenRoot(dir) and Lstat every component from files/ down, refusing any that is not a directory or a regular file. Add tests for a symlinked files and a symlinked files/<dir>.
  2. (should, may follow) The tree path only judges the named templates. Any other non-regular entry in files/ (a symlink, a fifo) passes module check and the gate, but pack refuses it, so the walk's build fails instead of the pull request. Judging files/ by pack's rule where the tree is read would close this.
  3. (note) TestTemplateTextsAreRefusedWhenTheTreeDoesNotPackToTheDigestTaken and TestTheMergeGateResolvesATemplatedModuleToItsInlineTwin call the functions directly, so they still pass with the wiring reverted. Other tests cover the wiring, so nothing needs to change here.
Verdict: CHANGES at f45e0a1de50828323532084196696a25a16da4f9 Checks I ran in my own worktree of this head: `sh merge-check.sh` with MESH_TEST_POSTGRES pointing at a throwaway PostgreSQL over TCP exited 0. That covers gofmt -l (empty), `CGO_ENABLED=0 go vet ./...` (clean), the whole suite under -race, and every listed store test passing. I also reverted the wiring locally (the builder's withTemplateTexts call, the gate's and module check's WithTemplatesFrom, and the registration guard). The builder, module check, merge gate ("judges a change to a template alone") and registration tests then fail, as they should. The points I was asked to judge: - (a) Byte-identical composition holds. A manifest without `template` takes no new path: `Templated()` is false, so `WithTemplatesFrom` returns it unchanged and Resolve skips the new block. The inline twin and the templated twin compose identically on both the archive path and the tree path. - (b) The reuse path is sound. `withTemplateTexts` packs files/ again with the same `pack` (internal/builder/builder.go:1238) and reads text only from bytes whose sha256 equals `built[at].Digest`. On a walk that takes a check's build, that is the digest it took, so the text cannot differ from the archive the mesh records. A tree that packs differently fails the build loudly. - (d) The author's statement is true. I checked it on the base branch: a manifest with `template` on a file parses, resolves and declares with `"template"` passed through to the node. mesh-host's `strictDecode` (DisallowUnknownFields) has no such field and refuses it. An older controller's merge gate would also pass such a manifest, so nothing automatic enforces the order. No catalogue manifest may use `template` until this controller and the builder built from it run on every build node, that is, after the release branch reaches main and its walk is done. Issue 655 (T2) should say so. - (e) Confirmed: the gate fills the templates (merge_gate.go:582) before `standInBuild`, and the filled manifest no longer names a template. Findings: 1. (must) internal/catalogue/template.go:200-211. `ReadTemplates` checks only the last path component with Lstat, so a symlinked `files/` or `files/<dir>` is followed out of the tree. I proved it: with `files -> /some/dir` and with `files/sub -> /some/dir`, `WithTemplatesFrom` returned the outside file's text with no error. This is the reader the merge gate (merge_gate.go:582) runs on a pull request's tree on a build node, and module check and `module add` use it too. It contradicts the package's own rule that symlinks are refused, and `pack` refuses them, so the gate can pass a tree the build will refuse. Fix: read through `os.OpenRoot(dir)` and Lstat every component from `files/` down, refusing any that is not a directory or a regular file. Add tests for a symlinked `files` and a symlinked `files/<dir>`. 2. (should, may follow) The tree path only judges the named templates. Any other non-regular entry in files/ (a symlink, a fifo) passes module check and the gate, but `pack` refuses it, so the walk's build fails instead of the pull request. Judging files/ by pack's rule where the tree is read would close this. 3. (note) `TestTemplateTextsAreRefusedWhenTheTreeDoesNotPackToTheDigestTaken` and `TestTheMergeGateResolvesATemplatedModuleToItsInlineTwin` call the functions directly, so they still pass with the wiring reverted. Other tests cover the wiring, so nothing needs to change here.
mesh-admin added 1 commit 2026-10-14 00:51:49 +00:00
Refuse templates read through a symlink anywhere under files/
mesh/delivery delivered
mesh/merge-gate head def3ccefe3c8 merged into trunk bfc79c64d989 — pass: the change touches no module of the mesh's graph
mesh/repo-check head def3ccefe3c8 merged into trunk bfc79c64d989 — pass: its merge-check.sh passed
def3ccefe3
A symlinked files/ or a symlinked directory under it let module check, the merge
gate and module add read a template from outside the repository tree, and a
symlink no template names passed them while the build's packing refuses it
(review of mesh-controller#313). Every entry under files/ must now be a plain
directory or file, and templates are read through an os.Root of the module's
directory.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Contributor

Verdict: APPROVE at def3ccefe3

This re-reviews the fix for finding 1 of comment 11963 (a symlinked files/ or files/<dir> was followed out of the tree).

Checks I ran in my own worktree of this head: gofmt -l printed nothing, and CGO_ENABLED=0 go vet ./... was clean. sh merge-check.sh exited 0 against a throwaway PostgreSQL over TCP (127.0.0.1:55613, started from a scratch initdb and stopped afterwards). Run verbosely, every template test passes, the gate tests included.

The fix:

  • ReadTemplates now reads through os.OpenRoot(dir) (root.Lstat and root.ReadFile). go.mod says go 1.26.0. os.OpenRoot and Root.Lstat exist since Go 1.24 and Root.ReadFile since 1.25, so any toolchain that can build this module has them, including the builder's Go bundle build and the merge gate's binary. My local toolchain is go1.27.1.
  • templatesTreeProblems refuses, by its path, a files/ that is not a plain directory and every entry under it that is neither a directory nor a regular file. I probed this myself. A FIFO is refused as "files/pipe is a named pipe". files -> <outside> is refused as "files/ is a symlink", and the os.Root read says "path escapes from parent" and returns no outside bytes. A direct root.ReadFile through the outside symlink returns nothing. The check runs only for a manifest that names a template (WithTemplatesFrom returns early when Templated() is false), so a module that has not moved is unaffected.
  • I reverted internal/catalogue/template.go to f45e0a1 and kept the tests. Then TestASymlinkAnywhereUnderFilesIsRefusedByName fails in all 3 cases. The module check case (files/elsewhere is a symlink) fails in TestModuleCheckReadsTemplatesFromTheTreeBesideTheManifest. The symlinked-files/ case fails in TestTheMergeGateJudgesAChangeToATemplateAlone, where the gate reported PASS. With the fix restored, all of them pass.

The earlier passing points still hold. def3cce touches only internal/catalogue/template.go and tests. TestATemplatedModuleComposesByteIdenticallyToItsInlineTwin and ...ReadFromItsTree... pass. withTemplateTexts still compares the re-packed sha256 with built[at].Digest (internal/builder/template.go). The gate still fills templates at merge_gate.go:582, before standInBuild at :608.

Findings (none blocks this approval):

  1. (should, may follow) internal/catalogue/template_test.go:262: no test covers a special file that is not a symlink, such as a FIFO. The code refuses it by name (I checked by probe). A FIFO case in TestASymlinkAnywhereUnderFilesIsRefusedByName would pin that down.
  2. (note) internal/catalogue/template.go:236 says the build's packing refuses a symlinked files/ itself. That is not quite true: pack (internal/builder/builder.go:1239) uses os.Stat on its root and follows a symlinked files/. A templated module's build is still refused, because builder.go:273 runs WithTemplatesFrom before packing. The comment could say that the build refuses it, not the packing.
  3. (note) With files -> ../real (a target inside the tree), the os.Root read reports "path escapes from parent" next to the correct "files/ is a symlink" message. The leading message is right, so this is harmless.
Verdict: APPROVE at def3ccefe3c8f26a4bb2dc6f95ba6efd9949bb6d This re-reviews the fix for finding 1 of comment 11963 (a symlinked `files/` or `files/<dir>` was followed out of the tree). Checks I ran in my own worktree of this head: `gofmt -l` printed nothing, and `CGO_ENABLED=0 go vet ./...` was clean. `sh merge-check.sh` exited 0 against a throwaway PostgreSQL over TCP (127.0.0.1:55613, started from a scratch initdb and stopped afterwards). Run verbosely, every template test passes, the gate tests included. The fix: - `ReadTemplates` now reads through `os.OpenRoot(dir)` (`root.Lstat` and `root.ReadFile`). go.mod says `go 1.26.0`. `os.OpenRoot` and `Root.Lstat` exist since Go 1.24 and `Root.ReadFile` since 1.25, so any toolchain that can build this module has them, including the builder's Go bundle build and the merge gate's binary. My local toolchain is go1.27.1. - `templatesTreeProblems` refuses, by its path, a `files/` that is not a plain directory and every entry under it that is neither a directory nor a regular file. I probed this myself. A FIFO is refused as "files/pipe is a named pipe". `files -> <outside>` is refused as "files/ is a symlink", and the os.Root read says "path escapes from parent" and returns no outside bytes. A direct `root.ReadFile` through the outside symlink returns nothing. The check runs only for a manifest that names a template (`WithTemplatesFrom` returns early when `Templated()` is false), so a module that has not moved is unaffected. - I reverted `internal/catalogue/template.go` to f45e0a1 and kept the tests. Then `TestASymlinkAnywhereUnderFilesIsRefusedByName` fails in all 3 cases. The module check case (`files/elsewhere is a symlink`) fails in `TestModuleCheckReadsTemplatesFromTheTreeBesideTheManifest`. The symlinked-`files/` case fails in `TestTheMergeGateJudgesAChangeToATemplateAlone`, where the gate reported PASS. With the fix restored, all of them pass. The earlier passing points still hold. def3cce touches only `internal/catalogue/template.go` and tests. `TestATemplatedModuleComposesByteIdenticallyToItsInlineTwin` and `...ReadFromItsTree...` pass. `withTemplateTexts` still compares the re-packed sha256 with `built[at].Digest` (internal/builder/template.go). The gate still fills templates at merge_gate.go:582, before `standInBuild` at :608. Findings (none blocks this approval): 1. (should, may follow) internal/catalogue/template_test.go:262: no test covers a special file that is not a symlink, such as a FIFO. The code refuses it by name (I checked by probe). A FIFO case in `TestASymlinkAnywhereUnderFilesIsRefusedByName` would pin that down. 2. (note) internal/catalogue/template.go:236 says the build's packing refuses a symlinked `files/` itself. That is not quite true: `pack` (internal/builder/builder.go:1239) uses `os.Stat` on its root and follows a symlinked `files/`. A templated module's build is still refused, because builder.go:273 runs `WithTemplatesFrom` before packing. The comment could say that the build refuses it, not the packing. 3. (note) With `files -> ../real` (a target inside the tree), the os.Root read reports "path escapes from parent" next to the correct "files/ is a symlink" message. The leading message is right, so this is harmless.
mesh-admin merged commit 617881c309 into feat/57-config-and-provisioning 2026-10-14 01:00:48 +00:00
mesh-admin deleted branch pkg/57-t1-template-files 2026-10-14 01:00:49 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: novox/mesh-controller#313