fix(diff): ignore preview env rows so they cannot shadow production (#85) #86

Merged
dan-claude-bot merged 1 commit from fix/env-preview-shadow into main 2026-07-16 16:39:34 +00:00
dan-claude-bot commented 2026-07-16 16:37:51 +00:00 (Migrated from github.com)

Fixes #85. Root cause of the symptom #78 chased.

The bug, in one line

cast declares production env on every write, and read production + preview. That asymmetry is the whole defect.

syncEnv sends is_preview: false on every bulk upsert. But GET /applications/{uuid}/envs does not return one row per key — it merges two parallel sets into one flat array:

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

The relations are complements split on is_preview, with a unique index per (key, resource, is_preview) — so the same key legitimately arrives twice. fetchEnv keyed by key alone and Object.fromEntries keeps the last, so cast diffed the manifest against whichever row Coolify happened to serialize last.

Confirmed on prod

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

cast read the trailing "false" twin and re-proposed a change that could never clear. The production rows are true — prod was correct all along; the diff was the broken thing.

Why this masqueraded as a stale read (#78)

Both rows are born equal — Coolify seeds a preview twin — and syncEnv only ever PATCHes the production row. So the two diverge for exactly the vars updated in place. Five prod flags flipped false→true re-proposed forever, while created-once vars (NODE_ENV, REPORTING_TZ, the ${domain:…} base-URLs) stayed clean because their twins still agreed — through the identical code path.

#78 spotted that pattern and reached for real_value. But real_value is an Attribute accessor recomputed from value on every read, so it cannot go stale — and the probe shows real_value == value on every row. Nothing was stale. cast was reading another deployment's value.

The fix

fetchEnv drops is_preview: true rows. A preview var is another deployment's value for the same name: not cast's to compare, and not cast's to write. Services and databases map a single set (their controllers don't merge), so it's a no-op for them.

Tests

  • The exact prod shape — production + preview twin, preview last → production wins.
  • Preview twin serialized FIRST → production still wins. The fix is drop preview, not take the first; ordering must not decide the answer, or the bug just moves.
  • A preview-only key is dropped entirely — it isn't production state, so it must not surface as an orphan/remove-candidate either.
  • Rows with no is_preview field (services/databases) are kept — absent must mean "keep", never "drop".

npm run check clean · 524 tests pass (npm run build && npm test, as CI runs).

Note on #79

Keep it. Comparing a manifest literal against real_value (an escaped/quoted rendering — is_literal'true'true) is wrong regardless, and it closes a latent secret-rotation hazard. It simply was never what was biting these five; its stated motivation should be re-pointed at this issue.

🤖 Generated with Claude Code

Fixes #85. Root cause of the symptom #78 chased. ## The bug, in one line **cast declares production env on every write, and read production + preview.** That asymmetry is the whole defect. `syncEnv` sends `is_preview: false` on every bulk upsert. But `GET /applications/{uuid}/envs` does **not** return one row per key — it merges two parallel sets into one flat array: ```php // ApplicationsController@envs, v4.1.2 $envs = $application->environment_variables->sortBy('id') ->merge($application->environment_variables_preview->sortBy('id')); ``` The relations are complements split on `is_preview`, with a unique index per `(key, resource, is_preview)` — so the **same key legitimately arrives twice**. `fetchEnv` keyed by `key` alone and `Object.fromEntries` keeps the **last**, so cast diffed the manifest against whichever row Coolify happened to serialize last. ## Confirmed on prod ```json { "key": "REPORTING_ENABLED", "value": "true", "real_value": "true", "is_preview": false } { "key": "BRAIN_ENABLED", "value": "true", "real_value": "true", "is_preview": false } { "key": "REPORTING_ENABLED", "value": "false", "real_value": "false", "is_preview": true } { "key": "BRAIN_ENABLED", "value": "false", "real_value": "false", "is_preview": true } ``` cast read the trailing `"false"` twin and re-proposed a `change` that could never clear. **The production rows are `true`** — prod was correct all along; the diff was the broken thing. ## Why this masqueraded as a stale read (#78) Both rows are born equal — Coolify seeds a preview twin — and `syncEnv` only ever PATCHes the **production** row. So the two diverge for *exactly* the vars updated in place. Five prod flags flipped `false→true` re-proposed forever, while created-once vars (`NODE_ENV`, `REPORTING_TZ`, the `${domain:…}` base-URLs) stayed clean because their twins still agreed — through the identical code path. #78 spotted that pattern and reached for `real_value`. But `real_value` is an `Attribute` accessor recomputed from `value` on every read, so it cannot go stale — and the probe shows `real_value == value` on **every** row. Nothing was stale. cast was reading **another deployment's value**. ## The fix `fetchEnv` drops `is_preview: true` rows. A preview var is another deployment's value for the same name: not cast's to compare, and not cast's to write. Services and databases map a single set (their controllers don't merge), so it's a no-op for them. ## Tests - The **exact prod shape** — production + preview twin, preview last → production wins. - **Preview twin serialized FIRST** → production still wins. The fix is *drop preview*, not *take the first*; ordering must not decide the answer, or the bug just moves. - A **preview-only** key is dropped entirely — it isn't production state, so it must not surface as an orphan/remove-candidate either. - Rows with **no `is_preview` field** (services/databases) are kept — absent must mean "keep", never "drop". `npm run check` clean · **524 tests pass** (`npm run build && npm test`, as CI runs). ## Note on #79 Keep it. Comparing a manifest literal against `real_value` (an *escaped/quoted* rendering — `is_literal` ⇒ `'true'` ≠ `true`) is wrong regardless, and it closes a latent secret-rotation hazard. It simply was never what was biting these five; its stated motivation should be re-pointed at this issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign in to join this conversation.
No reviewers
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#86
No description provided.