heavy-duty/cast

Fork 0

fix: never write an env var whose name Coolify injects itself (SOURCE_COMMIT, COOLIFY_*) #56

Merged

dan-claude-bot merged 1 commit from fix/reserved-env-names into main

2026-07-14 22:54:16 +00:00

dan-claude-bot commented

2026-07-14 22:30:01 +00:00

(Migrated from github.com)

The trap

Coolify injects SOURCE_COMMIT and the COOLIFY_* family into an application's runtime environment itself — and it skips its own injection of a name when the application already carries an env var of that name.

Verified against the real source, not the vendored OpenAPI spec (which is provably incomplete): app/Jobs/ApplicationDeploymentJob.php @ v4.1.2, generate_coolify_env_variables, lines 2994–3001:

if ($this->application->environment_variables->where('key', 'SOURCE_COMMIT')->isEmpty()) {
    if (! is_null($this->commit)) {
        $coolify_envs->put('SOURCE_COMMIT', $this->commit);
    } else {
        $coolify_envs->put('SOURCE_COMMIT', 'unknown');
    }
}

The same ->isEmpty() guard shape wraps each COOLIFY_* name at 3002–3028 (COOLIFY_FQDN, COOLIFY_URL, COOLIFY_BRANCH, COOLIFY_RESOURCE_UUID, COOLIFY_CONTAINER_NAME), and again on the preview branch at 2950–2984.

isEmpty() is asked of the collection of vars, never of the value. So an application-level SOURCE_COMMIT — even, especially, an empty one — does not merely fail to help: it suppresses the value Coolify would otherwise have provided. Presence, not value.

And it fails green. The deploy succeeds, the health check passes, the container runs — and the only symptom is that /version, which reads process.env.SOURCE_COMMIT at request time and which a production cutover is gated on, reports unknown. (D-266.)

Anything that writes env vars can set that trap, and cast is a thing that writes env vars. Before this PR, grep -rn "SOURCE_COMMIT\|COOLIFY_" src/ returned nothing.

The rule

Reserved names are SOURCE_COMMIT and anything matching ^COOLIFY_. A new src/reserved.ts owns the rule, the consequence sentence, and the refusal — and every place cast touches an env var honors it. The value of doing all of them together is that the rule is now a property of cast, not of one code path.

1. `resolve` / `apply` — REFUSE

The assertion sits in every manifest read in resolve.ts — desiredFromManifest, requiredSecrets, manifestResources — not in the verb that writes. So apply, diff, capture and inventory all refuse identically, before any write. Refusing in one and reporting in another would let capture write a store for a manifest apply is guaranteed to refuse: a green run that promises a red one.

It reads all template keys, not just the ${…} refs — a bare SOURCE_COMMIT= literal suppresses the injection exactly as well. Presence, not value, exactly as forbidden_var_patterns already does (envtemplate.ts:77).

2. `draft` / `capture` — NEVER COPY ONE

isProviderGenerated (draft.ts:113) was the only filter between a live var and a drafted manifest, and it recognizes SERVICE_* and datastore-connection names. SOURCE_COMMIT splits to [SOURCE, COMMIT] — no prefix, no datastore word, no connection word — so it was captured verbatim, with its live value, and drafting a working box reproduced the trap in the new box's manifest.

A third provenance, suppressed, joins captured and generated in the disposition machinery: kept out of the emitted template and out of the age store, its live value read into no artifact, listed in UNCAPTURED.md with the reason — the report that exists precisely so what cast declines to carry is said out loud rather than dropped — and shown in the disposition table (→ NOT COPIED (Coolify injects this itself)).

capture's classify carries the same assertion at the file. Unreachable through the CLI today (requiredSecrets refuses first) and kept anyway: the invariant is "cast never carries one", not "the CLI happens to check first", and a captured SOURCE_COMMIT would sit in the age store — the one artifact a reviewer cannot read. Same for syncEnv in cli.ts, the single function that puts an env var on the wire.

3. `diff` — PROMOTED OUT OF THE ORPHAN LIST

A reserved name on a live resource was reported as a remove-candidate orphan var — the category whose documented meaning is "apply never removes these; read them by eye" — so it read as cosmetic residue. It is not residue: it is an active suppression of a platform-provided value. It now prints as a FINDING with the consequence attached, and the report is not clean:

FINDING: application core carries env var SOURCE_COMMIT — DELETE IT (Coolify UI)
  SOURCE_COMMIT is injected by Coolify itself at runtime, and Coolify SKIPS its own
  injection when the resource already carries a var of that name
  (ApplicationDeploymentJob.php, v4.1.2). A var of this name — even an EMPTY one —
  SUPPRESSES the platform's value. The deploy stays green and /version reports "unknown".
  cast declares no such var and never will (it refuses a manifest that does),
  and `apply` never deletes — so this one is yours to remove, by hand, in the UI.
0 change(s), 0 orphan(s), 1 reserved-name FINDING(s)

The scan is over the live side, not over the resources the manifest declares, so it is also caught on an orphan resource — which no changes entry covers. apply never deletes holds unchanged: cast reports it, the human removes it in the UI.

4. `smoke` — asserted

smoke writes an env var too. Its probe names are exported and asserted outside the reserved space, so a future rename (COOLIFY_SMOKE_PROBE reads like the natural name) cannot set the trap on a live app — and note the delete at the end would restore nothing, since Coolify only injects at deploy time.

Where the rule lives, and why

In cast's own code, not in per-environment state. forbidden_var_patterns is the neighbouring rule and looks like the obvious home. It is not: that one is policy — an environment's own choice about its own vars, which prod may set harder than staging, and which lives in private state precisely so a product-side change cannot lower its own guard. This one is a fact about Coolify: true on every box, in every environment, for every project. There is no environment in which declaring SOURCE_COMMIT is correct, so there must be no file in which it can be permitted.

Nothing here touches cast's own config vars (COOLIFY_BASE_URL, COOLIFY_ACCESS_TOKEN, COOLIFY_READ_ONLY) — read from the operator's local instance file, never written to a resource. The namespace collides; the meaning does not.

Tests

19 new tests in test/reserved.test.ts, one section per path (313 total, up from 294 — all green under npm run check && npm run build && npm test). All unit tests against fakes; no live instance was contacted.

the rule: both shapes reserved; SOURCE_COMMIT_SHA, MY_SOURCE_COMMIT, COOLIFYISH, SERVICE_FQDN_* are not (over-reach is the one way this rule can do harm)
resolve: refuses, naming the resource + consequence; refuses an empty literal; refuses COOLIFY_*; refuses on the capture and inventory paths; leaves an ordinary manifest alone
capture: classify refuses rather than reading the live value into the store
draft: kept out of the template, kept out of the store, dispositioned suppressed, named in UNCAPTURED.md, and no env_template emitted at all when every var was reserved
diff: not a remove-candidate; not clean; renders as a FINDING; found on an orphan resource; none in structural mode; a clean box stays clean
smoke: probe names are outside the reserved space

Scope

Runtime half only. #46 is the build-time half — the "Include Source Commit in Build" toggle, which gates only the build arg and is not settable via the API. No warning from #46 is implemented here.

Noticed and deliberately not fixed (outside this diff): diff can only find a reserved name in full mode, since structural mode reads no env values at all — a box diffed with a standing read-only token will not surface one. Worth a follow-up, but it is a property of the token, not of this rule.

Closes #50.

## The trap Coolify injects `SOURCE_COMMIT` and the `COOLIFY_*` family into an application's runtime environment **itself** — and it **skips its own injection of a name when the application already carries an env var of that name**. Verified against the real source, not the vendored OpenAPI spec (which is provably incomplete): [`app/Jobs/ApplicationDeploymentJob.php` @ v4.1.2](https://github.com/coollabsio/coolify/blob/v4.1.2/app/Jobs/ApplicationDeploymentJob.php), `generate_coolify_env_variables`, **lines 2994–3001**: ```php if ($this->application->environment_variables->where('key', 'SOURCE_COMMIT')->isEmpty()) { if (! is_null($this->commit)) { $coolify_envs->put('SOURCE_COMMIT', $this->commit); } else { $coolify_envs->put('SOURCE_COMMIT', 'unknown'); } } ``` The same `->isEmpty()` guard shape wraps each `COOLIFY_*` name at **3002–3028** (`COOLIFY_FQDN`, `COOLIFY_URL`, `COOLIFY_BRANCH`, `COOLIFY_RESOURCE_UUID`, `COOLIFY_CONTAINER_NAME`), and again on the preview branch at **2950–2984**. `isEmpty()` is asked of the **collection of vars**, never of the value. So an application-level `SOURCE_COMMIT` — even, especially, an **empty** one — does not merely fail to help: it **suppresses** the value Coolify would otherwise have provided. **Presence, not value.** And it **fails green**. The deploy succeeds, the health check passes, the container runs — and the only symptom is that `/version`, which reads `process.env.SOURCE_COMMIT` at request time and which a production cutover is gated on, reports `unknown`. (D-266.) Anything that writes env vars can set that trap, and cast is a thing that writes env vars. Before this PR, `grep -rn "SOURCE_COMMIT\|COOLIFY_" src/` returned nothing. ## The rule Reserved names are `SOURCE_COMMIT` and anything matching `^COOLIFY_`. A new **`src/reserved.ts`** owns the rule, the consequence sentence, and the refusal — and every place cast touches an env var honors it. The value of doing all of them together is that the rule is now a property of **cast**, not of one code path. ### 1. `resolve` / `apply` — REFUSE The assertion sits in **every** manifest read in `resolve.ts` — `desiredFromManifest`, `requiredSecrets`, `manifestResources` — not in the verb that writes. So `apply`, `diff`, `capture` and `inventory` all refuse identically, before any write. Refusing in one and reporting in another would let `capture` write a store for a manifest `apply` is guaranteed to refuse: a green run that promises a red one. It reads **all** template keys, not just the `${…}` refs — a bare `SOURCE_COMMIT=` literal suppresses the injection exactly as well. Presence, not value, exactly as `forbidden_var_patterns` already does (`envtemplate.ts:77`). ### 2. `draft` / `capture` — NEVER COPY ONE `isProviderGenerated` (`draft.ts:113`) was the only filter between a live var and a drafted manifest, and it recognizes `SERVICE_*` and datastore-connection names. `SOURCE_COMMIT` splits to `[SOURCE, COMMIT]` — no prefix, no datastore word, no connection word — so it was **captured verbatim, with its live value**, and drafting a working box reproduced the trap in the new box's manifest. A third provenance, **`suppressed`**, joins `captured` and `generated` in the disposition machinery: kept out of the emitted template **and** out of the age store, its live value read into no artifact, listed in **UNCAPTURED.md** with the reason — the report that exists precisely so what cast declines to carry is said out loud rather than dropped — and shown in the disposition table (`→ NOT COPIED (Coolify injects this itself)`). `capture`'s `classify` carries the same assertion at the file. Unreachable through the CLI today (`requiredSecrets` refuses first) and kept anyway: the invariant is *"cast never carries one"*, not *"the CLI happens to check first"*, and a captured `SOURCE_COMMIT` would sit in the age store — the one artifact a reviewer cannot read. Same for `syncEnv` in `cli.ts`, the single function that puts an env var on the wire. ### 3. `diff` — PROMOTED OUT OF THE ORPHAN LIST A reserved name on a live resource was reported as a `remove-candidate` orphan var — the category whose documented meaning is *"apply never removes these; read them by eye"* — so it read as cosmetic residue. It is not residue: it is an active suppression of a platform-provided value. It now prints as a **FINDING** with the consequence attached, and the report is **not clean**: ``` FINDING: application core carries env var SOURCE_COMMIT — DELETE IT (Coolify UI) SOURCE_COMMIT is injected by Coolify itself at runtime, and Coolify SKIPS its own injection when the resource already carries a var of that name (ApplicationDeploymentJob.php, v4.1.2). A var of this name — even an EMPTY one — SUPPRESSES the platform's value. The deploy stays green and /version reports "unknown". cast declares no such var and never will (it refuses a manifest that does), and `apply` never deletes — so this one is yours to remove, by hand, in the UI. 0 change(s), 0 orphan(s), 1 reserved-name FINDING(s) ``` The scan is over the **live** side, not over the resources the manifest declares, so it is also caught on an orphan resource — which no `changes` entry covers. **`apply never deletes` holds unchanged**: cast reports it, the human removes it in the UI. ### 4. `smoke` — asserted `smoke` writes an env var too. Its probe names are exported and asserted outside the reserved space, so a future rename (`COOLIFY_SMOKE_PROBE` reads like the natural name) cannot set the trap on a live app — and note the delete at the end would restore nothing, since Coolify only injects at deploy time. ## Where the rule lives, and why **In cast's own code, not in per-environment state.** `forbidden_var_patterns` is the neighbouring rule and looks like the obvious home. It is not: that one is **policy** — an environment's own choice about its own vars, which prod may set harder than staging, and which lives in private state precisely so a product-side change cannot lower its own guard. This one is a **fact about Coolify**: true on every box, in every environment, for every project. There is no environment in which declaring `SOURCE_COMMIT` is correct, so there must be no file in which it can be permitted. Nothing here touches cast's own config vars (`COOLIFY_BASE_URL`, `COOLIFY_ACCESS_TOKEN`, `COOLIFY_READ_ONLY`) — read from the operator's local instance file, never written to a resource. The namespace collides; the meaning does not. ## Tests **19 new tests** in `test/reserved.test.ts`, one section per path (313 total, up from 294 — all green under `npm run check && npm run build && npm test`). All unit tests against fakes; no live instance was contacted. - the rule: both shapes reserved; `SOURCE_COMMIT_SHA`, `MY_SOURCE_COMMIT`, `COOLIFYISH`, `SERVICE_FQDN_*` are **not** (over-reach is the one way this rule can do harm) - resolve: refuses, naming the resource + consequence; refuses an **empty literal**; refuses `COOLIFY_*`; refuses on the capture and inventory paths; leaves an ordinary manifest alone - capture: `classify` refuses rather than reading the live value into the store - draft: kept out of the template, kept out of the store, dispositioned `suppressed`, named in UNCAPTURED.md, and no `env_template` emitted at all when every var was reserved - diff: **not** a `remove-candidate`; not clean; renders as a FINDING; found on an orphan resource; none in structural mode; a clean box stays clean - smoke: probe names are outside the reserved space ## Scope Runtime half only. #46 is the **build-time** half — the *"Include Source Commit in Build"* toggle, which gates only the build **arg** and is not settable via the API. No warning from #46 is implemented here. Noticed and deliberately **not** fixed (outside this diff): `diff` can only find a reserved name in **full** mode, since structural mode reads no env values at all — a box diffed with a standing read-only token will not surface one. Worth a follow-up, but it is a property of the token, not of this rule. Closes #50.

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#56

No description provided.