diff --git a/candy/plugin-build/buildkit.go b/candy/plugin-build/buildkit.go index 029bfc3..d4d1da3 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,65 @@ func pushOutput(ref string) []string { return []string{"--output", "type=image,name=" + ref + ",push=true"} } +// pushTargets selects the images a --push build publishes. +// +// ⚑ 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 +// - 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`, 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 + } + 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 + } + targets[b.Name] = true + } + return targets +} + +// importedNamespaceBox reports whether a resolved box name is qualified by an `import:` +// 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 { + ns, _, ok := spec.SplitNamespaceRef(name) + return ok && ns != "" +} + +// 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..acf2445 100644 --- a/candy/plugin-build/drive.go +++ b/candy/plugin-build/drive.go @@ -30,10 +30,17 @@ 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: 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. + 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 +87,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..dfbe756 --- /dev/null +++ b/candy/plugin-build/push_targets_test.go @@ -0,0 +1,186 @@ +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 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 ( + "testing" + + "github.com/opencharly/spec/spec" +) + +// pushPolicyReply is the resolved chain shape the failing build produced: the imported CachyOS +// 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-authored": {Name: "pod-crabbox-systemd-authored"}, + "pod-crabbox-systemd-base": {Name: "pod-crabbox-systemd-base", Auto: true}, + "pod-crabbox-systemd": {Name: "pod-crabbox-systemd"}, + }, + }, + } +} + +// 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 localBoxes() { + if !got[want] { + t.Errorf("push targets must include this project's box %q, got %v", want, got) + } + } + 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 localBoxes() { + if !got[want] { + t.Errorf("a full --push build must publish this project's box %q, got %v", want, got) + } + } + for _, foreign := range foreignBoxes() { + 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 := 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) + } + want := "type=image,name=" + requested.FullTag + ",push=true" + if got[1] != want { + t.Errorf("registry exporter: got %q want %q", got[1], want) + } + + 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) + } + + 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) + } +} + +// 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. +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 _, _, qualified := spec.SplitNamespaceRef(name); qualified { + t.Fatalf("push targets must not contain a foreign intermediate, got %q", name) + } + } +}