heavy-duty/cast

Fork 0

fetchEnv silently collapses duplicate env keys — a shadow row makes every in-place-updated var diff forever (cause of #78) #85

New issue

Closed

opened 2026-07-16 16:29:22 +00:00 by dan-claude-bot · 1 comment

dan-claude-bot commented

2026-07-16 16:29:22 +00:00

(Migrated from github.com)

Summary

GET /{resource}/{uuid}/envs does not return one row per key. It merges two disjoint sets — the production vars and the preview vars — into a single flat array:

// ApplicationsController@envs, v4.1.2
$envs = $application->environment_variables->sortBy('id')
          ->merge($application->environment_variables_preview->sortBy('id'));

// Application.php:967 — the two relations are complements, split on is_preview
public function environment_variables() { return $this->morphMany(...)->where('is_preview', false); }

Both collections are numerically keyed, so merge() appends (array_merge semantics on numeric keys) rather than overwriting — the response can legitimately contain two rows with the same key, one is_preview: false, one is_preview: true.

cast collapses that with last-wins, and has no notion of is_preview at all:

// src/cli.ts — fetchEnv
return Object.fromEntries(
  envs.map((e) => [e.key, { value: e.value, realValue: e.real_value }]),
);

Object.fromEntries keeps the last entry per key. So when both rows exist, cast diffs the manifest against whichever row Coolify happened to serialize last — and says nothing.

Why this is a defect on its own terms

cast declares production env. A preview var is a different deployment's value for the same name. Diffing against it produces drift cast can neither explain nor clear, and apply would then PATCH the production var to match something that was never about production. It fails in the shape this repo keeps legislating against (#12/#14/#17/#18): silently, with a confident-looking report — no line anywhere says "two rows carry this key."

It also quietly widens: every caller of fetchEnv inherits it — capture's store classification (flattenEnv), draft's scaffolding, and inventory's key listing.

CONFIRMED — this is the cause of #78

Probed against prod 2026-07-16. Two rows per key, and real_value tracks value on every one of them:

{ "key": "REPORTING_ENABLED", "value": "true",  "real_value": "true",  "is_literal": false, "is_shared": false }
{ "key": "BRAIN_ENABLED",     "value": "true",  "real_value": "true",  "is_literal": false, "is_shared": false }
{ "key": "REPORTING_ENABLED", "value": "false", "real_value": "false", "is_literal": false, "is_shared": false }
{ "key": "BRAIN_ENABLED",     "value": "false", "real_value": "false", "is_literal": false, "is_shared": false }

The production rows are "true" — the flags are genuinely on. Prod was correct all along; the diff was the thing that was wrong.
real_value == value on every row, which is exactly what the accessor predicts (is_literal: false ⇒ escapeEnvVariables("true") == "true"). #78's "stale real_value" theory is dead: nothing is stale.
cast's Object.fromEntries keeps the last row — the "false" one — and diffs it against the manifest's "true". Hence a phantom change that can never clear.

Why only the in-place-flipped vars re-propose

#78 spotted this and misattributed it. Both rows start equal at creation. cast apply PATCHes only the production row (false→true), leaving its twin at false — so the two diverge for exactly the vars that were updated in place. NODE_ENV, REPORTING_TZ and the ${domain:...} base-URLs were never flipped, so their rows still agree and last-wins picks a value that happens to match — which is why four other non-secret vars read clean through the identical code path.

The "something goes stale after an in-place PATCH" intuition was right. The stale thing is a second row, not a computed field.

Probe (reproduce / check any resource)

Dumps every entry for a key rather than the first, which is the whole point:

# core's uuid, and a token with sensitive read
COOLIFY=http://coolify-box:8000
TOKEN=...
UUID=<core-uuid>

curl -sS -H "Authorization: Bearer $TOKEN" "$COOLIFY/api/v1/applications/$UUID/envs" \
| jq '[.[] | select(.key=="REPORTING_ENABLED")]
      | {count: length,
         entries: [.[] | {id, value, real_value, is_literal, is_preview, is_shared}]}'

count: 2 is the bug. The remaining unknown is which flag distinguishes the two rows — include is_preview, since it decides the fix:

second row is_preview: true ⇒ it is a preview-deployment var; fetchEnv must drop preview rows.
both rows is_preview: false ⇒ Coolify is holding two production rows for one key, which cast must refuse to silently pick between (the backupNotCompared precedent).

Whole-resource sweep for any duplicated key:

curl -sS -H "Authorization: Bearer $TOKEN" "$COOLIFY/api/v1/applications/$UUID/envs" \
| jq 'group_by(.key) | map(select(length > 1))
      | map({key: .[0].key, rows: [.[] | {value, is_preview}]})'

What to do

fetchEnv: drop is_preview: true rows — cast declares production env; a preview var is another deployment's and is not cast's to compare or write
Failing that, at minimum refuse or report a duplicated key rather than silently taking the last — an unreadable answer must produce neither drift nor a clean bill (the backupNotCompared precedent)
Check the same collapse in flattenEnv / the capture + draft + inventory callers
Regression test: two rows for one key, one is_preview → the production row is what diffs, whatever the serialization order

Found while investigating #78 (root cause unknown after the real_value theory was disproved).

## Summary `GET /{resource}/{uuid}/envs` does not return one row per key. It **merges two disjoint sets** — the production vars and the *preview* vars — into a single flat array: ```php // ApplicationsController@envs, v4.1.2 $envs = $application->environment_variables->sortBy('id') ->merge($application->environment_variables_preview->sortBy('id')); ``` ```php // Application.php:967 — the two relations are complements, split on is_preview public function environment_variables() { return $this->morphMany(...)->where('is_preview', false); } ``` Both collections are numerically keyed, so `merge()` **appends** (array_merge semantics on numeric keys) rather than overwriting — the response can legitimately contain **two rows with the same `key`**, one `is_preview: false`, one `is_preview: true`. cast collapses that with last-wins, and has no notion of `is_preview` at all: ```ts // src/cli.ts — fetchEnv return Object.fromEntries( envs.map((e) => [e.key, { value: e.value, realValue: e.real_value }]), ); ``` `Object.fromEntries` keeps the **last** entry per key. So when both rows exist, cast diffs the manifest against *whichever row Coolify happened to serialize last* — and says nothing. ## Why this is a defect on its own terms cast declares **production** env. A preview var is a *different deployment's* value for the same name. Diffing against it produces drift cast can neither explain nor clear, and `apply` would then PATCH the production var to match something that was never about production. It fails in the shape this repo keeps legislating against (#12/#14/#17/#18): **silently**, with a confident-looking report — no line anywhere says "two rows carry this key." It also quietly widens: every caller of `fetchEnv` inherits it — `capture`'s store classification (`flattenEnv`), `draft`'s scaffolding, and `inventory`'s key listing. ## CONFIRMED — this is the cause of #78 Probed against prod 2026-07-16. **Two rows per key**, and `real_value` tracks `value` on every one of them: ```json { "key": "REPORTING_ENABLED", "value": "true", "real_value": "true", "is_literal": false, "is_shared": false } { "key": "BRAIN_ENABLED", "value": "true", "real_value": "true", "is_literal": false, "is_shared": false } { "key": "REPORTING_ENABLED", "value": "false", "real_value": "false", "is_literal": false, "is_shared": false } { "key": "BRAIN_ENABLED", "value": "false", "real_value": "false", "is_literal": false, "is_shared": false } ``` - The **production** rows are `"true"` — the flags are genuinely on. Prod was correct all along; the diff was the thing that was wrong. - `real_value == value` on every row, which is exactly what the accessor predicts (`is_literal: false` ⇒ `escapeEnvVariables("true") == "true"`). #78's "stale `real_value`" theory is dead: **nothing is stale**. - cast's `Object.fromEntries` keeps the **last** row — the `"false"` one — and diffs it against the manifest's `"true"`. Hence a phantom `change` that can never clear. ### Why only the in-place-flipped vars re-propose #78 spotted this and misattributed it. Both rows start **equal** at creation. `cast apply` PATCHes only the **production** row (false→true), leaving its twin at `false` — so the two diverge for exactly the vars that were updated in place. `NODE_ENV`, `REPORTING_TZ` and the `${domain:...}` base-URLs were never flipped, so their rows still agree and last-wins picks a value that happens to match — which is why four other non-secret vars read clean through the identical code path. The "something goes stale after an in-place PATCH" intuition was right. The stale thing is a **second row**, not a computed field. ## Probe (reproduce / check any resource) Dumps **every** entry for a key rather than the first, which is the whole point: ```bash # core's uuid, and a token with sensitive read COOLIFY=http://coolify-box:8000 TOKEN=... UUID=<core-uuid> curl -sS -H "Authorization: Bearer $TOKEN" "$COOLIFY/api/v1/applications/$UUID/envs" \ | jq '[.[] | select(.key=="REPORTING_ENABLED")] | {count: length, entries: [.[] | {id, value, real_value, is_literal, is_preview, is_shared}]}' ``` `count: 2` is the bug. The remaining unknown is **which flag distinguishes the two rows** — include `is_preview`, since it decides the fix: - second row `is_preview: true` ⇒ it is a *preview-deployment* var; `fetchEnv` must drop preview rows. - both rows `is_preview: false` ⇒ Coolify is holding **two production rows for one key**, which cast must refuse to silently pick between (the `backupNotCompared` precedent). Whole-resource sweep for any duplicated key: ```bash curl -sS -H "Authorization: Bearer $TOKEN" "$COOLIFY/api/v1/applications/$UUID/envs" \ | jq 'group_by(.key) | map(select(length > 1)) | map({key: .[0].key, rows: [.[] | {value, is_preview}]})' ``` ## What to do - [ ] `fetchEnv`: drop `is_preview: true` rows — cast declares production env; a preview var is another deployment's and is not cast's to compare or write - [ ] Failing that, at minimum **refuse or report** a duplicated key rather than silently taking the last — an unreadable answer must produce neither drift nor a clean bill (the `backupNotCompared` precedent) - [ ] Check the same collapse in `flattenEnv` / the `capture` + `draft` + `inventory` callers - [ ] Regression test: two rows for one key, one `is_preview` → the **production** row is what diffs, whatever the serialization order Found while investigating #78 (root cause unknown after the `real_value` theory was disproved).

dan-claude-bot commented

2026-07-16 16:44:53 +00:00

(Migrated from github.com)

Verified against prod — the fix works. cast diff --env prod --full, on a cast installed from main:

-  env REPORTING_ENABLED: change
-  env BRAIN_ENABLED: change
-  env BRANDED_EMAIL_ENABLED: change
-  env EMAIL_PREVIEW_ENABLED: change
-  env OPERATOR_SETTINGS_ENABLED: change

All five gone. The probe on the same box confirms the shape this issue describes: {value:"true", is_preview:false} and {value:"false", is_preview:true} per key, with real_value == value on every row.

One loose end worth confirming before this is fully closed

The same run also dropped two orphan lines that were present immediately before:

-  env INFRA_SMOKE_KEEP: live-only (orphan var — apply never removes)
-  env INFRA_SMOKE_PROBE: live-only (orphan var — apply never removes)

That was not predicted, and it should be explained rather than enjoyed. smoke.ts writes both with is_preview: false (lines 39-60), and create_bulk_envs honours the payload's is_preview without duplicating — so on the model above these are production rows, and the is_preview !== true filter cannot drop them.

Three readings, and only one of them is comfortable:

They really are is_preview: true on this box → the filter is right, and they had been reported as orphans in error all along.
They are is_preview: false → the filter is dropping production rows, i.e. #86 regressed the read it was meant to fix, and it should be reverted/narrowed.
They were deleted from the box between runs → unrelated to #86.

Probe, copy-paste:

COOLIFY=http://coolify-box:8000
TOKEN=...
UUID=<core-uuid>

curl -sS -H "Authorization: Bearer $TOKEN" "$COOLIFY/api/v1/applications/$UUID/envs" \
| jq 'map({key, value, is_preview})
      | { smoke:        map(select(.key | startswith("INFRA_SMOKE"))),
          preview_keys: map(select(.is_preview == true) | .key) }'

Leaving this open until that comes back — a fix whose side effects aren't understood isn't finished, and reading (1) would additionally mean cast smoke's canary has been sitting in the preview set, which is its own bug.

(Related, from the same run: #87 — with this fixed, Coolify's own SERVICE_* magic vars are all that keep a correct box from reading clean.)

**Verified against prod** — the fix works. `cast diff --env prod --full`, on a cast installed from `main`: ```diff - env REPORTING_ENABLED: change - env BRAIN_ENABLED: change - env BRANDED_EMAIL_ENABLED: change - env EMAIL_PREVIEW_ENABLED: change - env OPERATOR_SETTINGS_ENABLED: change ``` All five gone. The probe on the same box confirms the shape this issue describes: `{value:"true", is_preview:false}` **and** `{value:"false", is_preview:true}` per key, with `real_value == value` on every row. ## One loose end worth confirming before this is fully closed The same run **also** dropped two orphan lines that were present immediately before: ```diff - env INFRA_SMOKE_KEEP: live-only (orphan var — apply never removes) - env INFRA_SMOKE_PROBE: live-only (orphan var — apply never removes) ``` That was **not** predicted, and it should be explained rather than enjoyed. `smoke.ts` writes both with `is_preview: false` (lines 39-60), and `create_bulk_envs` honours the payload's `is_preview` without duplicating — so on the model above these are production rows, and the `is_preview !== true` filter cannot drop them. Three readings, and only one of them is comfortable: 1. They really are `is_preview: true` on this box → the filter is right, and they had been reported as orphans **in error** all along. 2. They are `is_preview: false` → **the filter is dropping production rows**, i.e. #86 regressed the read it was meant to fix, and it should be reverted/narrowed. 3. They were deleted from the box between runs → unrelated to #86. Probe, copy-paste: ```bash COOLIFY=http://coolify-box:8000 TOKEN=... UUID=<core-uuid> curl -sS -H "Authorization: Bearer $TOKEN" "$COOLIFY/api/v1/applications/$UUID/envs" \ | jq 'map({key, value, is_preview}) | { smoke: map(select(.key | startswith("INFRA_SMOKE"))), preview_keys: map(select(.is_preview == true) | .key) }' ``` Leaving this open until that comes back — a fix whose *side effects* aren't understood isn't finished, and reading (1) would additionally mean `cast smoke`'s canary has been sitting in the preview set, which is its own bug. (Related, from the same run: #87 — with this fixed, Coolify's own `SERVICE_*` magic vars are all that keep a correct box from reading clean.)

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/cast#85

No description provided.