Honor all runtime_config fields over the workload API

[jhrozek]

Summary

• The workload API's request type is *templates.RuntimeConfig and swagger publishes all four of its fields, but the service layer only ever copied builder_image and additional_packages. A caller POSTing runtime_config.build_with got 201 Created and a workload built with unconstrained dependencies — silently, which is exactly the failure build_with exists to prevent. runtime_env was dropped the same way.
• The root cause was three hand-written, field-by-field copies of a struct onto itself. They were complete when written, then runtime_env (Add runtime-stage environment variables to protocol Dockerfiles #5801) and build_with (Rename --uv-with to --build-with; reject constraints on unsupported ecosystems #6116) were added and each silently stopped being carried — the "parallel types that drift" anti-pattern from .claude/rules/go-style.md. The first commit moves copy and merge onto the type that owns the fields (Clone, WithOverrides, ValidateFor, IsEmpty); the second makes the API use them and honor all four fields.
• Plumbing the missing fields naively would have traded a silent drop for an opaque 500: the build-constraint rejection lived deep in the imageRetriever path, and pkg/api/errors/handler.go scrubs any ≥500 body to bare status text. So validation moved. runtimeConfigForImageBuild now merges and validates before the retriever, and that error is wrapped in retriever.ErrInvalidRunConfig — coded 400 and returned intact.
• Carrying the fields through to responses exposed a round trip that was already broken for builder_image/additional_packages: a protocol-built workload persists the built image, so GET returned a runtime_config that PUT rejected with 400, and GET → edit → PUT could not save an existing workload back.

Fixes #6210

Type of change

• Bug fix

Test plan

• Unit tests (task test)
• Linting (task lint-fix)

Every new test was confirmed to fail against the pre-fix code before being accepted. That mattered here: an earlier iteration of the round-trip fix asserted only HTTP 200 and passed while silently erasing the workload's build configuration. Asserting a mutation's status code without asserting its effect certifies the wrong thing.

Changes

File	Change
pkg/container/templates/runtime_config.go	Clone, WithOverrides, ValidateFor, IsEmpty on the type; GetDefaultRuntimeConfig returns a detached value
pkg/runner/protocol.go	Uses the new methods; hand-rolled mergeRuntimeConfig/mergeEnvMaps deleted; constraint check extracted
pkg/api/v1/workload_service.go	All four fields honored; validateRuntimeConfig/isValidRuntimePackageName deleted for the type's own validation; ValidateFor placed so violations return 400; inert-echo exception; shared normalizeRuntimeConfig
pkg/api/v1/workload_types.go	runtime_config field doc rewritten; response path clones
pkg/api/v1/workloads.go	Create and update endpoints document runtime_config applicability
docs/server/*	Regenerated (endpoint descriptions only — the fields were always published)
*_test.go	Round-trip, policy-gate visibility, remote echo, padded-image echo, corrupt-state, aliasing, dedupe and rejection coverage

Does this introduce a user-facing change?

Yes, on the REST API:

• runtime_config.build_with and runtime_env are honored instead of silently discarded. A build_with on npx:///go:// returns 400 with a readable message rather than being ignored or surfacing as Internal Server Error.
• additional_packages no longer contains duplicates when a requested package is already a transport default.
• Package names starting with . or _ are now rejected at the API. They were accepted and then failed at build time — the deleted validator was missing the leading-character check packageNamePattern enforces.
• GET → edit → PUT of a protocol-built workload succeeds instead of returning 400, and the workload's build configuration is preserved rather than erased.
• A globally configured runtime_configs.<tt>.build_with survives a per-request runtime_config that sets other fields. Previously any request-supplied runtime_config discarded the global constraint, because the API layers the config file as the merge base and the merge replaced BuildWith outright.

And one CLI-visible message change: the build-constraint rejection now says build_with rather than naming the --build-with flag, since the same message is reachable from the API, the TUI and the config file. It also gains a prefix identifying where the value came from (invalid runtime config override: / ... in config file for <tt>: / ... default runtime config for <tt>:).

Special notes for reviewers

Why validation sits in runtimeConfigForImageBuild, not in the builder. This placement is load-bearing and there is a comment saying so. Errors from that call are wrapped in retriever.ErrInvalidRunConfig and reach the client as a 400; the identical failure inside imageRetriever is scrubbed to Internal Server Error. Moving the check "closer to where it's used" silently reverts the fix with tests still green.

The echo exception is narrow by design. It requires an exact match of runtime_config and image and URL against persisted state — comparing only runtime_config would let a request change the image to nginx:latest while echoing the old config and bypass the guard. Both operands are normalized, so a workload persisted with an untrimmed builder_image still round-trips. The request's runtime_config is never cleared, so the policy gate always evaluates the real config; a test asserts that directly rather than relying on convention.

On the "policy enforcement happens after untrusted dependencies execute" concern, if it comes up: that ordering is pre-existing and unchanged here. The build-then-policy sequence is byte-identical on the merge base, the CLI path is untouched, and a strictly stronger primitive was already API-reachable — builder_image is copied from the request body and validated only by nameref.ParseReference, then lands in FROM {{.RuntimeConfig.BuilderImage}}, i.e. unconstrained pre-gate build-time root execution with BuildEnv and COPY .netrc in scope. What this PR adds is narrower: build_with is uvx-only, length-capped, and allowlisted against quotes, $, ;, backslash and parens. Worth fixing at the shared choke point in retriever.ResolveMCPServer so CLI, API, TUI and the upgrade applier are covered at once — filed separately, since it is a cross-cutting change to a downstream-implemented interface.

Known follow-ups, deliberately not in scope:

• loadRuntimeConfig does not implement the three-tier precedence (flags > user config > built-ins) that docs/runtime-version-customization.md documents — it skips the config file entirely when an override is present. Pre-existing, and fixing it changes what existing setups build, so it needs its own decision.
• build_with and runtime_env are absent from docs/arch/05-runconfig-and-permissions.md and docs/runtime-version-customization.md. Pre-existing debt from Add runtime-stage environment variables to protocol Dockerfiles #5801 and Rename --uv-with to --build-with; reject constraints on unsupported ecosystems #6116.

[codecov]

Codecov Report

❌ Patch coverage is 88.88889% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.06%. Comparing base (af68771) to head (bd3da88).

Files with missing lines	Patch %	Lines
pkg/container/templates/runtime_config.go	92.15%	2 Missing and 2 partials ⚠️
pkg/runner/protocol.go	50.00%	3 Missing and 1 partial ⚠️
pkg/api/v1/workload_service.go	93.61%	3 Missing ⚠️
pkg/api/v1/workload_types.go	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6214      +/-   ##
==========================================
+ Coverage   73.00%   73.06%   +0.06%     
==========================================
  Files         745      745              
  Lines       78747    78764      +17     
==========================================
+ Hits        57487    57551      +64     
+ Misses      17248    17188      -60     
- Partials     4012     4025      +13     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[samuv]

Thanks for the thorough fix. The API now preserves and validates all RuntimeConfig fields, keeps GET to edit round trips working for built images, and centralizes cloning and merging on RuntimeConfig. I did not find a blocking correctness or security issue.

One merge-readiness note: this branch is currently 78 commits behind main, and the substantive CI checks shown on the head commit ran on August 5. Please refresh the branch and rerun CI before merging.

Checklist:

• Tests: Comprehensive unit and API regression coverage. Existing head checks are green, with the freshness note above.
• Docs: Swagger output was regenerated and endpoint behavior is documented.
• Registry impact: None.
• Security: Merged runtime configuration is validated before the retriever, and echoed configuration remains visible to the policy gate.
• Backwards compatibility: The change fixes silent field loss and preserves existing GET to edit workflows.

[jhrozek]

Thanks for the review! Fixed both nits in bd3da88, replied on each thread. Also rebased onto current main and pushed — head is now bd3da88. Full unit suite passes except TestIntegration_SSEGetConnectionSurvivesWriteTimeout in pkg/vmcp/server, which is unrelated to this change (SSE write-timeout test, nothing touching workload_service.go or runtime_config.go) and looks like a pre-existing flake on main.

[samuv]

Thanks for addressing both review comments. The follow-up commit cleanly adopts immutable assignment and removes the reflection-based guard, and the branch is now current with main. The completed checks are green, with the remaining E2E and operator jobs still running. Looks good to me.