feat(claude): host-side marketplace cloning for private plugin repos

[abezzub-dr]

Summary

• Clone marketplace repos on the host (where git credentials exist) instead of inside the Docker build context
• Copy marketplace files into the build context and write known_marketplaces.json directly
• claude plugin install finds plugins from the local filesystem — no network needed during build
• Falls back gracefully to build-time claude plugin marketplace add if host clone fails

This enables private plugin marketplaces (e.g., company-internal GitHub repos) to work in moat containers without leaking credentials into the Docker image.

Changes

1. internal/providers/claude/marketplace.go — CloneMarketplace, CollectMarketplaceFiles, GenerateKnownMarketplaces
2. internal/providers/claude/dockerfile.go — Handle PreCloned marketplaces with COPY + known_marketplaces.json
3. internal/deps/dockerfile.go — Pass through ExtraContextFiles to build context
4. internal/run/manager.go — Clone marketplaces before image build, merge into context

Test plan

• Unit tests pass: go test ./internal/providers/claude/ -v
• Existing deps tests pass: go test ./internal/deps/ -v
• Configure a private marketplace in moat.yaml and verify plugins install

[dpup]

The design is sound and solves the real problem (private repos need host credentials, not build-time credentials). But there's one P1 that blocks merge and several P2s worth fixing.

🔴 P1 — Ref field is silently ignored (blocks merge)

config.MarketplaceSpec has a Ref string field with a config test that asserts it round-trips. CloneMarketplace accepts a ref parameter and passes it as --branch. But manager.go:1258 hardcodes it to "":

clonedDir, cloneErr := claude.CloneMarketplace(ctx, m.Repo, "")

A user who sets ref: v2.0 in moat.yaml silently gets the default branch. Additionally, Ref isn't included in the image hash, so changing the ref wouldn't even trigger a rebuild.

Either wire it through (propagate Ref from config → MarketplaceConfig → CloneMarketplace, and include it in ImageTag), or remove the field from the schema until it's implemented. The current state is a documented API that does nothing.

🟡 P2 — Should fix before merge

time.Now() in GenerateKnownMarketplaces (marketplace.go:128)

Every call produces a different known-marketplaces.json, invalidating Docker's layer cache on every build even when nothing changed. The rest of the system is carefully content-addressed — this breaks that invariant. Use a fixed constant ("1970-01-01T00:00:00Z") or the repo's last commit timestamp.

Clone loop runs before ImageExists check (manager.go:1256 vs 1359)

The clone + collect loop runs unconditionally. If the image is already cached (the common case), all marketplace repos are cloned and all files are read into memory for nothing. Gate the clone loop on !exists, consistent with how the build step itself is gated.

ExtraContextFiles inside if ScriptName != "" (deps/dockerfile.go:214–219)

known-marketplaces.json is only merged into the build context inside the ScriptName guard. Today this always holds (pre-cloned marketplaces imply plugins imply a script), but the coupling is implicit. If a future caller sets ExtraContextFiles without a script, the files are silently dropped while the Dockerfile still has COPY directives referencing them — a confusing build failure. Move the ExtraContextFiles merge outside the condition.

Validate marketplace names in manager.go before CollectMarketplaceFiles (manager.go:~1262)

The name from ExtraKnownMarketplaces is used to construct build-context map keys ("marketplaces/" + name + "/") before validMarketplaceName is checked. Validation happens later in GenerateDockerfileSnippet. Check validMarketplaceName at the top of the loop in manager.go and skip early on invalid names — don't let an invalid name influence the build-context map at all.

Add --no-recurse-submodules (marketplace.go:86)

git clone --depth 1 doesn't prevent submodule initialization. A malicious marketplace with submodules could trigger hooks on the host. Add --no-recurse-submodules to the clone args.

🔵 P3 — Nice to have

• validPreClonedPath allows .. — the regex ^[a-zA-Z0-9._/-]+$ permits traversal sequences. Not exploitable today (PreCloned is always "marketplaces/" + validatedName), but misleadingly permissive. Consider ^[a-zA-Z0-9_-]+(/[a-zA-Z0-9_.-]+)*$ or an explicit strings.Contains(m.PreCloned, "..") guard.
• fmt.Sprintf with no format args at dockerfile.go:138 — plain string literal suffices.
• ref parameter — if not wiring it up now, remove it from CloneMarketplace's signature until there's a concrete use case with a config field behind it.
• File size guard in CollectMarketplaceFiles — no bound on per-file or aggregate size. A large binary in a marketplace repo loads entirely into memory. Worth a size check with a log.Warn and skip.

[abezzub-dr]

@dpup I think I addressed all comments, can you look again pls?

Regarding the ref: it looks like marketplace config already had the definition of the ref, but it was not used anywhere in the code. So for now I removed it. Is that okay? Or was it used somewhere that I don't see?

Summary of changes by Claude Code:

Performance

• Marketplace cloning is now gated on !exists so cached images skip it entirely (ed6e0a2)

Security & safety

• Marketplace names are validated at the top of the clone loop, before they can influence build-context map keys (f0b3fed)
• Path traversal (..) is explicitly rejected in pre-cloned marketplace paths, with a test (9078e68, 519d978)
• --no-recurse-submodules added to marketplace clone to prevent malicious submodule hooks (7653f18)

Correctness

• time.Now() replaced with repo commit timestamp in known_marketplaces.json so Docker layer cache isn't invalidated on every build (56eda55)
• ExtraContextFiles merged outside the ScriptName guard so they aren't silently dropped when there's no plugin script (805ef3a)
• Unused Ref field removed from MarketplaceSpec — it was never wired through, so ref: in moat.yaml was silently ignored (d31367a)

[abezzub-dr]

There was a conflict with the main branch, so I had to rebase the PR. Hopefully that does not make the review too complicated. This is the first commit you have not reviewed yet: 8e4b254

[dpup]

Review comments on the host-side marketplace cloning logic.

[dpup]

If CollectMarketplaceFiles succeeds but the repo is empty (0 files), contextFiles stays nil/empty but PreCloned is still set here. Later, the len(marketplaceContextFiles) > 0 guard at line 1430 skips the Dockerfile regeneration — so the COPY directives never appear. But PreCloned is set, so GenerateDockerfileSnippet filters this marketplace into the preCloned slice and skips the marketplace add command for it. Net effect: the marketplace is silently skipped entirely.

Consider guarding this assignment on len(files) > 0, or treating an empty clone as a failure that falls back to build-time cloning.

[dpup]

This closure mutates claudeMarketplaces elements via pointer (m.PreCloned = ..., m.CommitTime = ...) as a side effect, but the signature only advertises returning cleanups and contextFiles. The hidden mutation is what makes the second GenerateDockerfile call produce different output — it only works because the caller knows to re-call GenerateDockerfile after this runs.

Consider either documenting the mutation in the comment/signature (e.g. returning the modified configs) or restructuring so the data flow is explicit.

[dpup]

This call site is ~120 lines from the closure definition at line 1300. The distance makes it hard to follow the data flow, especially since the closure mutates claudeMarketplaces as a side effect that the code here depends on (the second GenerateDockerfile call). Consider making cloneMarketplacesOnHost a standalone function, or moving the definition closer to this call site.

[abezzub-dr]

I found one more issue with registering marketplace when building a fresh image, I will try to add a fix for it. It seems to not work only with Apple containers

[dpup]

Thanks for the contribution! This is shipping in v0.5.0.