fix(services): merge environment on partial service update instead of replacing - #620
Merged
Hydralerne merged 2 commits intoAug 18, 2026
Merged
Conversation
… replacing
PATCH /api/projects/:id/services/:serviceId ran patch.environment through
unmaskEnv, which builds its result from the incoming keys alone. A body naming
one variable therefore deleted every variable it did not mention, so the
documented "send only what changes" (docs/api/services.mdx) silently destroyed
the rest of the map.
That is worse here than on the endpoint's other collection fields because
`environment` is the only one MASKED on read, and env-reveal is deliberately
kept off the automation surface: a raw-API or MCP client cannot see what it is
about to overwrite and cannot read the values back afterwards. It is the same
pairing the project-level PUT /:id/env was removed for ("it could wipe/corrupt
masked secrets"); service-env masking then arrived without the merge path.
mergeServiceEnv merges onto the stored map with the same absent/null/present
triad as mergeAdvanced, plus the sentinel arm only a masked field needs: an
absent key is kept, an explicit null removes one, `null` clears the map, `{}`
changes nothing, and an echoed •••••••• restores the stored value. unmaskEnv is
untouched and keeps its whole-map semantics for the six writers that genuinely
own the whole set (create, compose sync, project-crud, migration, deploy) —
each rebuilds the map from an upstream spec and must be able to drop a variable
the spec no longer declares.
`environment` is nullable on update ONLY, for the reason already spelled out
for domain/customDomain in the same object literal: absent means "keep", so
"clear" needs its own spelling. Create and sync keep Record<string,string>.
Env-tab saves now name their removed keys explicitly via serviceEnvPatch, which
lives next to the other dashboard payload builders and is tested there — a
client that submits only its current rows can no longer no-op a deletion while
reporting success.
Fixes oblien#619
…lly read it The merge semantics are only useful if a client can discover them, and the two surfaces that mislead are the ones nobody edits by hand: the MCP tool schema and the public API reference. The PATCH route is MCP-exposed and mcp-tools.ts emits UpdateServiceBody verbatim as the tool inputSchema, so an agent saw `environment` as a bare string map behind the description "Update a service's configuration." with destructiveHint: false — nothing in the advertised surface distinguished merge from replace, and after the merge change an agent's natural revocation attempt (omit the key) returns 200 OK while the credential stays injected. The route description and the field's TypeBox description now spell out the absent/null/clear triad, the •••••••• round-trip, and — the part that is easy to get wrong in the other direction — that every OTHER field on this endpoint still replaces wholesale, so ports/volumes/dependsOn/publicEndpoints need the complete list. docs/api/services.mdx gets the same table under "Update a service"; its create-side type stays Record<string,string>, which is what create accepts. service-schema-env pins both halves of the asymmetry. Moving the nullable field into ComposeFieldsBlock (the obvious dedupe) would teach create and sync to accept nulls that unmaskEnv writes through as dropped keys, and the update override only holds because it is declared after the spread — reordering the object literal reverts it with no other symptom. The value type is also pinned UNBOUNDED: capping it on update alone, as the first cut did at 10000, makes a long value (CA chain, base64 kubeconfig) creatable via POST or /sync and then permanently un-PATCH-able.
Hydralerne
force-pushed
the
fix/service-env-partial-merge
branch
from
August 18, 2026 16:13
677279c to
3a20904
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #619
Summary
Fixes an issue where
PATCH /api/projects/:id/services/:serviceIdcompletely replaces a service'senvironmentmap when a partial environment update is supplied, wiping all other variables and stored secrets on the service.Changes
secret-env.ts): AddedmergeServiceEnv(stored, incoming)implementing JSON-merge patch semantics with masked secret restoration (ENV_MASK) and key deletion via explicitnull.service.schema.ts): WidenedUpdateServiceBody.environmentschema to allow nullable values per key (Type.Union([Type.String(), Type.Null()])) and a nullable environment record for full clearance.service.service.ts): UpdatedupdateServiceto callmergeServiceEnvonpatch.environmentinstead ofunmaskEnv.ServiceDetailPanel.tsx): UpdatedhandleSaveEnvto compute deleted keys as explicitnullin the patch payload so UI deletions are transmitted properly to the backend.apps/api/test/lib/secret-env.test.tsformergeServiceEnvtesting partial merges, updates, explicit null deletions, masked sentinels, full clearance, and undefined/empty payloads.apps/api/test/modules/services/service-update-env.test.tsverifyingupdateServicepreserves untouched variables during partial updates.