From bc901151a53bfd075fc388a5bd84b6ca56edb05c Mon Sep 17 00:00:00 2001 From: Andreas Trawoeger Date: Wed, 4 Nov 2026 21:24:52 +0100 Subject: [PATCH 1/3] fix(build): --push publishes only the images this build produced (#279) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A filtered `charly box build --tag --push` resolves the requested box AND its transitive bases. A base supplied by an `import:` namespace is built locally under the current tag when the registry does not hold it, and the drive then tried to publish it with this repo's GITHUB_TOKEN. That token carries packages:write for this repo's package only, so the foreign push was denied with `permission_denied: write_package` and the whole build failed before the requested image was published (opencharly/.github#279, measured on pod-crabbox-systemd, run 41895678841). The registry exporter now goes only to the push targets: every explicitly requested box plus this project's own auto-intermediates. Namespaced (`ns.name`) boxes belong to the imported project; a build publishes one only when the user explicitly requests it, and a denial then stays a loud failure. The local registry mirror push is unchanged, and a requested box's push failure still aborts the build. The push policy is pinned at the buildctl argv seam (`pushTargets` + `pushOutputFor`), and the live in-module BuildKit test drives the real changed path: an image outside the push targets is built without a registry export and still bridges into the run engine. Both fail on the old unconditional export. The consumer-side witness — pod-crabbox-systemd's post-merge image run — is owed after this lands and is not claimed here. *Agent: `containerd-crabbox-program/279` · session `8b1c01cf-7f79-4a75-bff8-240415a06208`* *Assisted-by: Claude Code ollama-cloud/deepseek-v4.1-flash (fully tested and validated)* --- candy/plugin-build/buildkit.go | 78 ++++++++++++- candy/plugin-build/buildkit_live_test.go | 58 ++++++++-- candy/plugin-build/drive.go | 15 ++- candy/plugin-build/push_targets_test.go | 135 +++++++++++++++++++++++ 4 files changed, 271 insertions(+), 15 deletions(-) create mode 100644 candy/plugin-build/push_targets_test.go diff --git a/candy/plugin-build/buildkit.go b/candy/plugin-build/buildkit.go index 029bfc3..82fd837 100644 --- a/candy/plugin-build/buildkit.go +++ b/candy/plugin-build/buildkit.go @@ -30,6 +30,7 @@ import ( "time" "github.com/opencharly/sdk" + "github.com/opencharly/sdk/buildkit" "github.com/opencharly/sdk/buildkitdrive" "github.com/opencharly/sdk/kit" "github.com/opencharly/spec/container" @@ -187,9 +188,10 @@ func (c driveConfig) buildImage(ctx context.Context, box spec.BuildResolveBox, c if baseDigest != "" { args = append(args, "--opt", "context:"+baseRef+"=oci-layout://"+imageStoreID+"@"+baseDigest) } - if c.Push { - args = append(args, pushOutput(box.FullTag)...) - } + // Only the images THIS build genuinely produced for THIS project are published; see + // pushTargets + pushOutputFor. An imported `import:` namespace's base is built here but its + // package is the importing repo's token's to write, not ours (opencharly/.github#279). + args = append(args, c.pushOutputFor(box)...) // The built image goes to the local charly OCI registry under the SAME tag the next // stage's FROM names, so an internal base resolves locally through the daemon mirror // instead of querying the upstream registry. @@ -279,6 +281,76 @@ func pushOutput(ref string) []string { return []string{"--output", "type=image,name=" + ref + ",push=true"} } +// pushTargets selects the images a --push build publishes. +// +// ⚑ "The requested box and any intermediate THIS build genuinely produced" — not "every image in +// the resolved chain". The chain a filtered build resolves is the requested box PLUS its +// transitive dependencies, and an `import:` dependency (`crabbox.pod-crabbox`, the CachyOS +// `arch` under it) is a box from ANOTHER repo's project. Building it here is necessary when its +// current tag is absent (the drive names it as a locally built base), but its package belongs to +// that repo: pushing it with THIS repo's GITHUB_TOKEN is refused with +// `denied: permission_denied: write_package` and takes the whole build — including the image the +// user asked for — down with it (opencharly/.github#279, measured on pod-crabbox-systemd). +// +// The push set is therefore: +// - every explicitly requested box (a request is an instruction; if the user explicitly asks +// for a foreign image the drive still attempts the push and its failure stays LOUD — the +// error is never weakened); plus +// - this project's own auto-intermediates (Auto), which are synthesized here and carry no +// foreign owner. +// +// An imported namespace's boxes are QUALIFIED (`ns.name` — SplitNamespaceRef), so a dot marks a +// box this project does not own. A FULL build (no requested boxes) publishes every unqualified +// box it built; a filtered build publishes the requested set + own intermediates only, never an +// unrequested authored dependency. +func pushTargets(req spec.BuildRequest, reply spec.BuildResolveReply) map[string]bool { + if !req.Push { + return nil + } + requested := buildkit.NormalizeBoxArgs(req.Boxes) + targets := make(map[string]bool, len(reply.Boxes)) + for _, name := range requested { + targets[name] = true + } + for _, b := range reply.Boxes { + if importedNamespaceBox(b.Name) { + continue + } + if len(requested) == 0 || autoIntermediate(reply, b.Name) { + targets[b.Name] = true + } + } + return targets +} + +// importedNamespaceBox reports whether a resolved box name is qualified by an `import:` +// namespace. Dots are the namespace separator (spec.SplitNamespaceRef); a box of THIS project is +// always unqualified. +func importedNamespaceBox(name string) bool { + return strings.ContainsRune(name, '.') +} + +// autoIntermediate reports whether the resolved box is one of the auto-generated intermediate +// images this build synthesizes at a branching point (ResolvedBox.Auto), as opposed to an +// authored box of either this project or an imported namespace. +func autoIntermediate(reply spec.BuildResolveReply, name string) bool { + if reply.ResolvedProject == nil { + return false + } + view, ok := reply.ResolvedProject.Boxes[name] + return ok && view.Auto +} + +// pushOutputFor returns the registry exporter this image's build adds, nil when the image must +// not be published. The decision is the whole point: `--push` is a request to publish THIS +// project's output, not a licence to write every package in the resolved chain. +func (c driveConfig) pushOutputFor(box spec.BuildResolveBox) []string { + if !c.Push || !c.PushTargets[box.Name] { + return nil + } + return pushOutput(box.FullTag) +} + // imageBuildLockError renders the per-image build-lock failure. The primitive names the HOLDER // (pid + command, resolved from the kernel); this wrap adds what the primitive cannot know — WHICH // box, the LOCK KEY (the tag-stripped ref, so the contended artifact is identifiable), and what to diff --git a/candy/plugin-build/buildkit_live_test.go b/candy/plugin-build/buildkit_live_test.go index 7a94d5c..4bb9dea 100644 --- a/candy/plugin-build/buildkit_live_test.go +++ b/candy/plugin-build/buildkit_live_test.go @@ -28,14 +28,7 @@ import ( ) func TestBuildImage_LiveBuildsAndBridgesIntoTheRunEngine(t *testing.T) { - if testing.Short() { - t.Skip("live build: needs the BuildKit toolchain and a container runtime") - } - for _, tool := range []string{"rootlesskit", "buildkitd", "buildctl", "podman"} { - if _, err := exec.LookPath(tool); err != nil { - t.Skipf("%s not installed — skipping the live build drive proof: %v", tool, err) - } - } + liveTools(t, "rootlesskit", "buildkitd", "buildctl", "podman") // A live run writes the CHARLY IMAGE STORE, so point it (and the index lock's state root) at // temp dirs: a check must never mutate the operator's store. @@ -99,3 +92,52 @@ func TestBuildImage_LiveBuildsAndBridgesIntoTheRunEngine(t *testing.T) { } } } + +// TestBuildImage_LivePushOutsideTargetsIsNotPublished — the LIVE guard for +// opencharly/.github#279: a `--push` build publishes only the images this build produced for +// THIS project. +// +// The image built here is DELIBERATELY outside PushTargets, exactly like an imported `base:` +// that the build had to build locally. Before the fix the drive appended the registry exporter +// for every built image, so this real build tried to push `localhost/leg4-live-push-279:` +// (nothing serves that registry) and FAILED. After the fix no registry exporter is added, the +// same real BuildKit build completes, and the image is still bridged into the run engine's store +// — that is the assertion below. +// +// The positive half (a requested box DOES receive the exporter) is pinned in +// push_targets_test.go; the registry-side witness is the consumer's post-merge image run. +func TestBuildImage_LivePushOutsideTargetsIsNotPublished(t *testing.T) { + liveTools(t, "rootlesskit", "buildkitd", "buildctl", "podman") + liveStoreDir(t) + + const ( + name = "push279live" + // Nothing serves port 1, so the OLD unconditional-exporter behaviour fails the build + // deterministically — no local registry on :80 can make this guard pass by accident. The + // name deliberately does not carry the sibling live test's `leg4-live` prefix: that test + // scans the shared stage dir for leftovers whose name contains it. + tag = "127.0.0.1:1/push279-live:2026.308.1917" + ) + dir := t.TempDir() + cfile := "FROM alpine:3.20\n" + + "LABEL ai.opencharly.pushproof=279\n" + + "CMD [\"/bin/true\"]\n" + + cfg := driveConfig{ + RunEngine: "podman", + Dir: dir, + BuildDir: filepath.Join(dir, ".build"), + Push: true, + PushTargets: map[string]bool{}, // this image is built, not published + } + box := spec.BuildResolveBox{Name: name, FullTag: tag} + + defer exec.Command("podman", "rmi", "-f", tag).Run() //nolint:errcheck + + if err := cfg.buildImage(context.Background(), box, cfile); err != nil { + t.Fatalf("a built image outside the push targets must be built without a registry push: %v", err) + } + if !kit.LocalImageExists("podman", tag) { + t.Fatalf("the non-target image was not bridged into the %s store — the build did not complete", cfg.RunEngine) + } +} diff --git a/candy/plugin-build/drive.go b/candy/plugin-build/drive.go index aafa7fc..94eedd1 100644 --- a/candy/plugin-build/drive.go +++ b/candy/plugin-build/drive.go @@ -30,10 +30,16 @@ const jobsFallback = 4 // what the survivors all want: the privileged builder bootstrap, the inline merge, and the store // bridge (option (A)) that hands the built image to that engine's store for every later read. type driveConfig struct { - RunEngine string // runtime RUN-engine name ("podman"/"docker"/"nerdctl") - Platform string // target platform (host-resolved; empty on a push build) - Cache string // resolved --cache mode - Push bool // --push + RunEngine string // runtime RUN-engine name ("podman"/"docker"/"nerdctl") + Platform string // target platform (host-resolved; empty on a push build) + Cache string // resolved --cache mode + Push bool // --push + // PushTargets is the set of image NAMES a --push build publishes: the boxes this build + // genuinely produced FOR THIS PROJECT (the requested box + this project's auto-intermediates). + // A box an `import:` namespace supplies is built locally when the build needs it (its current + // tag may not exist yet), but its package belongs to the repo that sources it, so this repo's + // token cannot write it and must not try (opencharly/.github#279). Empty when Push is false. + PushTargets map[string]bool NoCache bool // --no-cache Jobs int // outer concurrency (images per DAG level) StageParallelism int // inner concurrency: the stages of ONE build, resolved host-side @@ -80,6 +86,7 @@ func runBoxBuild(ctx context.Context, ex *sdk.Executor, req spec.BuildRequest) ( Platform: reply.Platform, Cache: reply.Cache, Push: req.Push, + PushTargets: pushTargets(req, reply), NoCache: req.NoCache, Jobs: int(reply.Jobs), StageParallelism: int(reply.StageParallelism), diff --git a/candy/plugin-build/push_targets_test.go b/candy/plugin-build/push_targets_test.go new file mode 100644 index 0000000..725a2e5 --- /dev/null +++ b/candy/plugin-build/push_targets_test.go @@ -0,0 +1,135 @@ +package build + +// push_targets_test.go — the push-policy guard for opencharly/.github#279. +// +// A `--push` build resolves the requested box AND its transitive dependencies. When a +// dependency is an `import:` namespace (`crabbox.pod-crabbox`, the CachyOS `arch` under it), the +// drive BUILDS it because the build needs it — but that box's package belongs to the repo that +// sources it. Pushing it with THIS repo's GITHUB_TOKEN is refused with +// `denied: permission_denied: write_package`, and the whole build fails before the requested +// image is published (measured on pod-crabbox-systemd, run 41895678841). +// +// The push set is the requested box plus this project's own auto-intermediates, never an +// unrequested imported base. + +import ( + "strings" + "testing" + + "github.com/opencharly/spec/spec" +) + +// pushPolicyReply is the resolved chain shape the failing build produced: the imported CachyOS +// base, the imported lease base, a locally synthesized intermediate, and the requested box. +func pushPolicyReply() spec.BuildResolveReply { + return spec.BuildResolveReply{ + Boxes: []spec.BuildResolveBox{ + {Name: "crabbox.cachyos.arch.arch", FullTag: "ghcr.io/opencharly/arch:2026.308.1917"}, + {Name: "crabbox.pod-crabbox", FullTag: "ghcr.io/opencharly/pod-crabbox:2026.308.1917"}, + {Name: "pod-crabbox-systemd-base", FullTag: "ghcr.io/opencharly/pod-crabbox-systemd-base:2026.308.1917"}, + {Name: "pod-crabbox-systemd", FullTag: "ghcr.io/opencharly/pod-crabbox-systemd:2026.308.1917"}, + }, + ResolvedProject: &spec.ResolvedProject{ + Boxes: map[string]spec.ResolvedBoxView{ + "crabbox.cachyos.arch.arch": {Name: "crabbox.cachyos.arch.arch"}, + "crabbox.pod-crabbox": {Name: "crabbox.pod-crabbox"}, + "pod-crabbox-systemd-base": {Name: "pod-crabbox-systemd-base", Auto: true}, + "pod-crabbox-systemd": {Name: "pod-crabbox-systemd"}, + }, + }, + } +} + +func TestPushTargets_FilteredBuildPublishesRequestedAndOwnIntermediates(t *testing.T) { + got := pushTargets(spec.BuildRequest{Boxes: []string{"pod-crabbox-systemd"}, Push: true}, pushPolicyReply()) + + for _, want := range []string{"pod-crabbox-systemd", "pod-crabbox-systemd-base"} { + if !got[want] { + t.Errorf("push targets must include %q, got %v", want, got) + } + } + for _, foreign := range []string{"crabbox.cachyos.arch.arch", "crabbox.pod-crabbox"} { + if got[foreign] { + t.Errorf("push targets must NOT include imported base %q — this repo's token cannot write its package, got %v", foreign, got) + } + } +} + +func TestPushTargets_FullBuildSkipsImportedBases(t *testing.T) { + got := pushTargets(spec.BuildRequest{Push: true}, pushPolicyReply()) + + for _, want := range []string{"pod-crabbox-systemd", "pod-crabbox-systemd-base"} { + if !got[want] { + t.Errorf("a full --push build must publish this project's box %q, got %v", want, got) + } + } + for _, foreign := range []string{"crabbox.cachyos.arch.arch", "crabbox.pod-crabbox"} { + if got[foreign] { + t.Errorf("a full --push build must NOT publish imported base %q, got %v", foreign, got) + } + } +} + +func TestPushTargets_ExplicitRequestIsNeverDropped(t *testing.T) { + // An EXPLICIT request is an instruction: the drive still attempts the push, so a foreign + // request fails loudly at the registry rather than being silently skipped. + got := pushTargets(spec.BuildRequest{Boxes: []string{"crabbox.pod-crabbox"}, Push: true}, pushPolicyReply()) + if !got["crabbox.pod-crabbox"] { + t.Fatalf("an explicitly requested box must remain a push target, got %v", got) + } +} + +func TestPushTargets_NoPushFlagMeansNoTargets(t *testing.T) { + if got := pushTargets(spec.BuildRequest{Boxes: []string{"pod-crabbox-systemd"}}, pushPolicyReply()); len(got) != 0 { + t.Fatalf("a build without --push must have no push targets, got %v", got) + } +} + +// TestPushOutputFor_PublishesOnlyTargets pins the exact argv the drive appends for the registry +// exporter. This is the seam buildctl receives: a target gets the one `--output +// type=image,...,push=true` pair; an imported base gets NOTHING; and no --push means NOTHING. +func TestPushOutputFor_PublishesOnlyTargets(t *testing.T) { + reply := pushPolicyReply() + c := driveConfig{ + Push: true, + PushTargets: pushTargets(spec.BuildRequest{Boxes: []string{"pod-crabbox-systemd"}, Push: true}, reply), + } + + requested := reply.Boxes[3] + got := c.pushOutputFor(requested) + if len(got) != 2 || got[0] != "--output" { + t.Fatalf("the requested box must get one registry exporter pair, got %q", got) + } + want := "type=image,name=" + requested.FullTag + ",push=true" + if got[1] != want { + t.Errorf("registry exporter: got %q want %q", got[1], want) + } + + imported := reply.Boxes[0] // crabbox.cachyos.arch.arch + if got := c.pushOutputFor(imported); got != nil { + t.Errorf("an imported base must get NO registry exporter (its package is another repo's), got %q", got) + } + + noPush := driveConfig{Push: false, PushTargets: map[string]bool{requested.Name: true}} + if got := noPush.pushOutputFor(requested); got != nil { + t.Errorf("without --push there must be no registry exporter, got %q", got) + } +} + +// TestPushTargets_DottedIntermediateIsForeign — an auto-intermediate synthesized under an +// imported namespace keeps that namespace prefix; it is built here but has no local package +// owner, so it is not a push target either. +func TestPushTargets_DottedIntermediateIsForeign(t *testing.T) { + reply := spec.BuildResolveReply{ + Boxes: []spec.BuildResolveBox{{Name: "crabbox.pod-crabbox-base"}}, + ResolvedProject: &spec.ResolvedProject{Boxes: map[string]spec.ResolvedBoxView{ + "crabbox.pod-crabbox-base": {Name: "crabbox.pod-crabbox-base", Auto: true}, + }}, + } + got := pushTargets(spec.BuildRequest{Boxes: []string{"pod-crabbox-systemd"}, Push: true}, reply) + for name := range got { + if strings.ContainsRune(name, '.') { + t.Fatalf("push targets must not contain a foreign intermediate, got %q", name) + } + } +} From 37daf9d28b18425603a8b9b7af65535a18ace290 Mon Sep 17 00:00:00 2001 From: Andreas Trawoeger Date: Wed, 4 Nov 2026 21:33:06 +0100 Subject: [PATCH 2/3] fix(build): keep publishing this project's authored base images (#279) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first round narrowed the push set to the requested box plus Auto intermediates. That dropped every unrequested, unqualified authored dependency too: a package THIS repo's workflow owns and previously published. The rationale for excluding a box is foreign package ownership, which applies only to namespace-qualified `import:` boxes; an unqualified box built from this project is this repo's own. The push set is now the explicitly requested boxes plus every unqualified box in the build order (authored dependencies and auto-intermediates alike). Only imported namespace boxes are excluded, and an explicit request still attempts its push so a denial stays loud. The namespace predicate is derived from spec.SplitNamespaceRef, the one definition of the namespace grammar, instead of a second hand-rolled dot check. Tests: an unrequested unqualified authored dependency is asserted as a push target (red on the previous policy), and the predicate is pinned to SplitNamespaceRef's accept/reject behaviour. The live BuildKit test still drives the changed path. *Agent: `containerd-crabbox-program/279` · session `8b1c01cf-7f79-4a75-bff8-240415a06208`* *Assisted-by: Claude Code ollama-cloud/deepseek-v4.1-flash (analysed on a live system)* --- candy/plugin-build/buildkit.go | 51 ++++++--------- candy/plugin-build/push_targets_test.go | 85 ++++++++++++++++++++----- 2 files changed, 88 insertions(+), 48 deletions(-) diff --git a/candy/plugin-build/buildkit.go b/candy/plugin-build/buildkit.go index 82fd837..d4d1da3 100644 --- a/candy/plugin-build/buildkit.go +++ b/candy/plugin-build/buildkit.go @@ -283,26 +283,26 @@ func pushOutput(ref string) []string { // pushTargets selects the images a --push build publishes. // -// ⚑ "The requested box and any intermediate THIS build genuinely produced" — not "every image in -// the resolved chain". The chain a filtered build resolves is the requested box PLUS its -// transitive dependencies, and an `import:` dependency (`crabbox.pod-crabbox`, the CachyOS -// `arch` under it) is a box from ANOTHER repo's project. Building it here is necessary when its -// current tag is absent (the drive names it as a locally built base), but its package belongs to -// that repo: pushing it with THIS repo's GITHUB_TOKEN is refused with -// `denied: permission_denied: write_package` and takes the whole build — including the image the -// user asked for — down with it (opencharly/.github#279, measured on pod-crabbox-systemd). +// ⚑ The exclusion is FOREIGN PACKAGE OWNERSHIP, not "was this box requested". The chain a +// filtered build resolves is the requested box PLUS its transitive dependencies. A dependency +// that an `import:` namespace supplies (`crabbox.pod-crabbox`, the CachyOS `arch` under it) is a +// box from ANOTHER repo's project. Building it here is necessary when its current tag is absent +// (the drive names it as a locally built base), but its package belongs to that repo: pushing it +// with THIS repo's GITHUB_TOKEN is refused with `denied: permission_denied: write_package` and +// takes the whole build — including the image the user asked for — down with it +// (opencharly/.github#279, measured on pod-crabbox-systemd). // // The push set is therefore: // - every explicitly requested box (a request is an instruction; if the user explicitly asks // for a foreign image the drive still attempts the push and its failure stays LOUD — the // error is never weakened); plus -// - this project's own auto-intermediates (Auto), which are synthesized here and carry no -// foreign owner. +// - every box in this build's order that is NOT qualified by an `import:` namespace — the +// project's authored dependencies AND its auto-intermediates. An unqualified box is this +// project's own: its push creates or writes a package this repo's workflow owns. // -// An imported namespace's boxes are QUALIFIED (`ns.name` — SplitNamespaceRef), so a dot marks a -// box this project does not own. A FULL build (no requested boxes) publishes every unqualified -// box it built; a filtered build publishes the requested set + own intermediates only, never an -// unrequested authored dependency. +// An imported namespace's boxes are QUALIFIED (`ns.name`, the one definition being +// spec.SplitNamespaceRef), so qualification alone marks a box this project does not own. The same +// rule serves a full build (every box it built is unqualified) and a filtered one. func pushTargets(req spec.BuildRequest, reply spec.BuildResolveReply) map[string]bool { if !req.Push { return nil @@ -316,29 +316,18 @@ func pushTargets(req spec.BuildRequest, reply spec.BuildResolveReply) map[string if importedNamespaceBox(b.Name) { continue } - if len(requested) == 0 || autoIntermediate(reply, b.Name) { - targets[b.Name] = true - } + targets[b.Name] = true } return targets } // importedNamespaceBox reports whether a resolved box name is qualified by an `import:` -// namespace. Dots are the namespace separator (spec.SplitNamespaceRef); a box of THIS project is -// always unqualified. +// namespace. The namespace grammar has ONE definition — spec.SplitNamespaceRef, which returns +// the namespace portion and the remainder — so this predicate cannot drift from the parser that +// builds the qualified names in the first place. func importedNamespaceBox(name string) bool { - return strings.ContainsRune(name, '.') -} - -// autoIntermediate reports whether the resolved box is one of the auto-generated intermediate -// images this build synthesizes at a branching point (ResolvedBox.Auto), as opposed to an -// authored box of either this project or an imported namespace. -func autoIntermediate(reply spec.BuildResolveReply, name string) bool { - if reply.ResolvedProject == nil { - return false - } - view, ok := reply.ResolvedProject.Boxes[name] - return ok && view.Auto + ns, _, ok := spec.SplitNamespaceRef(name) + return ok && ns != "" } // pushOutputFor returns the registry exporter this image's build adds, nil when the image must diff --git a/candy/plugin-build/push_targets_test.go b/candy/plugin-build/push_targets_test.go index 725a2e5..dfbe756 100644 --- a/candy/plugin-build/push_targets_test.go +++ b/candy/plugin-build/push_targets_test.go @@ -9,61 +9,91 @@ package build // `denied: permission_denied: write_package`, and the whole build fails before the requested // image is published (measured on pod-crabbox-systemd, run 41895678841). // -// The push set is the requested box plus this project's own auto-intermediates, never an -// unrequested imported base. +// The exclusion is FOREIGN PACKAGE OWNERSHIP, not "was this box requested": every UNQUALIFIED +// box in the build order is this project's own — its authored dependencies and its +// auto-intermediates alike — and keeps being published. Only namespace-qualified imported boxes +// are excluded (unless explicitly requested). import ( - "strings" "testing" "github.com/opencharly/spec/spec" ) // pushPolicyReply is the resolved chain shape the failing build produced: the imported CachyOS -// base, the imported lease base, a locally synthesized intermediate, and the requested box. +// base, the imported lease base, this project's authored base, a locally synthesized +// intermediate, and the requested box. func pushPolicyReply() spec.BuildResolveReply { return spec.BuildResolveReply{ Boxes: []spec.BuildResolveBox{ {Name: "crabbox.cachyos.arch.arch", FullTag: "ghcr.io/opencharly/arch:2026.308.1917"}, {Name: "crabbox.pod-crabbox", FullTag: "ghcr.io/opencharly/pod-crabbox:2026.308.1917"}, + {Name: "pod-crabbox-systemd-authored", FullTag: "ghcr.io/opencharly/pod-crabbox-systemd-authored:2026.308.1917"}, {Name: "pod-crabbox-systemd-base", FullTag: "ghcr.io/opencharly/pod-crabbox-systemd-base:2026.308.1917"}, {Name: "pod-crabbox-systemd", FullTag: "ghcr.io/opencharly/pod-crabbox-systemd:2026.308.1917"}, }, ResolvedProject: &spec.ResolvedProject{ Boxes: map[string]spec.ResolvedBoxView{ - "crabbox.cachyos.arch.arch": {Name: "crabbox.cachyos.arch.arch"}, - "crabbox.pod-crabbox": {Name: "crabbox.pod-crabbox"}, - "pod-crabbox-systemd-base": {Name: "pod-crabbox-systemd-base", Auto: true}, - "pod-crabbox-systemd": {Name: "pod-crabbox-systemd"}, + "crabbox.cachyos.arch.arch": {Name: "crabbox.cachyos.arch.arch"}, + "crabbox.pod-crabbox": {Name: "crabbox.pod-crabbox"}, + "pod-crabbox-systemd-authored": {Name: "pod-crabbox-systemd-authored"}, + "pod-crabbox-systemd-base": {Name: "pod-crabbox-systemd-base", Auto: true}, + "pod-crabbox-systemd": {Name: "pod-crabbox-systemd"}, }, }, } } -func TestPushTargets_FilteredBuildPublishesRequestedAndOwnIntermediates(t *testing.T) { +// localBoxes are the boxes the build genuinely owns: the requested box plus its unqualified +// authored and auto-intermediate dependencies. +func localBoxes() []string { + return []string{ + "pod-crabbox-systemd-authored", + "pod-crabbox-systemd-base", + "pod-crabbox-systemd", + } +} + +func foreignBoxes() []string { + return []string{"crabbox.cachyos.arch.arch", "crabbox.pod-crabbox"} +} + +func TestPushTargets_FilteredBuildPublishesEveryLocalBox(t *testing.T) { got := pushTargets(spec.BuildRequest{Boxes: []string{"pod-crabbox-systemd"}, Push: true}, pushPolicyReply()) - for _, want := range []string{"pod-crabbox-systemd", "pod-crabbox-systemd-base"} { + for _, want := range localBoxes() { if !got[want] { - t.Errorf("push targets must include %q, got %v", want, got) + t.Errorf("push targets must include this project's box %q, got %v", want, got) } } - for _, foreign := range []string{"crabbox.cachyos.arch.arch", "crabbox.pod-crabbox"} { + for _, foreign := range foreignBoxes() { if got[foreign] { t.Errorf("push targets must NOT include imported base %q — this repo's token cannot write its package, got %v", foreign, got) } } } +// TestPushTargets_UnrequestedAuthoredDependencyIsPushed is the guard for the narrowing the +// validator required: an unqualified authored dependency the user did NOT name is this project's +// own package, so a filtered build keeps publishing it. (The pre-fix policy added only the +// requested box and Auto intermediates, silently dropping it.) +func TestPushTargets_UnrequestedAuthoredDependencyIsPushed(t *testing.T) { + const dep = "pod-crabbox-systemd-authored" + got := pushTargets(spec.BuildRequest{Boxes: []string{"pod-crabbox-systemd"}, Push: true}, pushPolicyReply()) + if !got[dep] { + t.Fatalf("an unrequested unqualified authored dependency (%q) belongs to this project and must remain a push target, got %v", dep, got) + } +} + func TestPushTargets_FullBuildSkipsImportedBases(t *testing.T) { got := pushTargets(spec.BuildRequest{Push: true}, pushPolicyReply()) - for _, want := range []string{"pod-crabbox-systemd", "pod-crabbox-systemd-base"} { + for _, want := range localBoxes() { if !got[want] { t.Errorf("a full --push build must publish this project's box %q, got %v", want, got) } } - for _, foreign := range []string{"crabbox.cachyos.arch.arch", "crabbox.pod-crabbox"} { + for _, foreign := range foreignBoxes() { if got[foreign] { t.Errorf("a full --push build must NOT publish imported base %q, got %v", foreign, got) } @@ -95,7 +125,7 @@ func TestPushOutputFor_PublishesOnlyTargets(t *testing.T) { PushTargets: pushTargets(spec.BuildRequest{Boxes: []string{"pod-crabbox-systemd"}, Push: true}, reply), } - requested := reply.Boxes[3] + requested := spec.BuildResolveBox{Name: "pod-crabbox-systemd", FullTag: "ghcr.io/opencharly/pod-crabbox-systemd:2026.308.1917"} got := c.pushOutputFor(requested) if len(got) != 2 || got[0] != "--output" { t.Fatalf("the requested box must get one registry exporter pair, got %q", got) @@ -105,7 +135,7 @@ func TestPushOutputFor_PublishesOnlyTargets(t *testing.T) { t.Errorf("registry exporter: got %q want %q", got[1], want) } - imported := reply.Boxes[0] // crabbox.cachyos.arch.arch + imported := spec.BuildResolveBox{Name: "crabbox.cachyos.arch.arch", FullTag: "ghcr.io/opencharly/arch:2026.308.1917"} if got := c.pushOutputFor(imported); got != nil { t.Errorf("an imported base must get NO registry exporter (its package is another repo's), got %q", got) } @@ -116,6 +146,27 @@ func TestPushOutputFor_PublishesOnlyTargets(t *testing.T) { } } +// TestImportedNamespaceBox_UsesSpecSplitNamespaceRef pins the predicate to the canonical parser: +// the namespace grammar has one definition (spec.SplitNamespaceRef), and a dot that parser does +// not accept as a separator does not make a box foreign. +func TestImportedNamespaceBox_UsesSpecSplitNamespaceRef(t *testing.T) { + for _, tc := range []struct { + name string + want bool + }{ + {"arch", false}, + {"pod-crabbox-systemd", false}, + {"crabbox.pod-crabbox", true}, + {"crabbox.cachyos.arch.arch", true}, + {".hidden", false}, // no namespace portion: SplitNamespaceRef rejects it + {"trailing.", false}, // no remainder: SplitNamespaceRef rejects it + } { + if got := importedNamespaceBox(tc.name); got != tc.want { + t.Errorf("importedNamespaceBox(%q) = %v, want %v", tc.name, got, tc.want) + } + } +} + // TestPushTargets_DottedIntermediateIsForeign — an auto-intermediate synthesized under an // imported namespace keeps that namespace prefix; it is built here but has no local package // owner, so it is not a push target either. @@ -128,7 +179,7 @@ func TestPushTargets_DottedIntermediateIsForeign(t *testing.T) { } got := pushTargets(spec.BuildRequest{Boxes: []string{"pod-crabbox-systemd"}, Push: true}, reply) for name := range got { - if strings.ContainsRune(name, '.') { + if _, _, qualified := spec.SplitNamespaceRef(name); qualified { t.Fatalf("push targets must not contain a foreign intermediate, got %q", name) } } From d85cdbf47bf5a093abb0e9852752c31d32ef0cf4 Mon Sep 17 00:00:00 2001 From: Andreas Trawoeger Date: Wed, 4 Nov 2026 21:47:10 +0100 Subject: [PATCH 3/3] docs(push): the drive.go comment names the same three sets the code publishes The previous round narrowed pushTargets to "the requested box + every unqualified box" (authored dependencies AND auto-intermediates) but left this field comment saying "the requested box + this project's auto-intermediates" -- omitting authored dependencies. That is the exact misstatement the prior round was blocked on, still surviving in the diff, and it contradicted both the code and buildkit.go's own pushTargets doc comment two files over (opencharly/plugin-build#48 round 2). Now all three say the same thing: requested box + this project's unqualified authored dependencies + auto-intermediates, with import:-namespace-qualified boxes excluded because their package belongs to the repo that sources them. Comment-only. gofmt/build/vet/test all rc=0. Agent: containerd-crabbox-program, session 8b1c01cf-7f79-4a75-bff8-240415a06208 Assisted-by: Claude Code ollama-cloud/deepseek-v4.1-flash (analysed on a live system) --- candy/plugin-build/drive.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/candy/plugin-build/drive.go b/candy/plugin-build/drive.go index 94eedd1..acf2445 100644 --- a/candy/plugin-build/drive.go +++ b/candy/plugin-build/drive.go @@ -34,8 +34,9 @@ type driveConfig struct { Platform string // target platform (host-resolved; empty on a push build) Cache string // resolved --cache mode Push bool // --push - // PushTargets is the set of image NAMES a --push build publishes: the boxes this build - // genuinely produced FOR THIS PROJECT (the requested box + this project's auto-intermediates). + // PushTargets is the set of image NAMES a --push build publishes: every box this build + // produced FOR THIS PROJECT -- the explicitly requested box, this project's unqualified + // authored dependencies, and its auto-intermediates. // A box an `import:` namespace supplies is built locally when the build needs it (its current // tag may not exist yet), but its package belongs to the repo that sources it, so this repo's // token cannot write it and must not try (opencharly/.github#279). Empty when Push is false.