From 4de89c360bcf8495e3056537354fc7b60e480103 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Mon, 20 Jul 2026 11:43:14 +0000 Subject: [PATCH] feat: an application can declare HTTP basic auth, and apply sets it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UNCAPTURED.md has said since it existed that Basic Auth is "carried as raw container labels. cast's manifest has no field for them, so a rebuilt resource is UNPROTECTED where the original was not." For applications that is a cast vocabulary gap, not a Coolify one: is_http_basic_auth_enabled, http_basic_auth_username and http_basic_auth_password are in both the create and the PATCH allowlists at v4.1.2 (ApplicationsController.php:914, :2368). An application now declares `basic_auth: { enabled, username, password }`, with the password a store ${REF} and only a ${REF} — the schema refuses a literal, because a manifest is a committed file. It resolves out of the environment's age store through the same mechanism every env-template ref uses, and a missing or empty entry fails before anything is written. Managing it is opt-in (the is_static rule): an unconditional `false` would have the first apply after this ships strip protection off every app enabled by hand in the UI. Enabling without both credentials is refused at parse time and again at the wire — Coolify's own rule (:2446-2463), enforced before the request rather than discovered as a mid-run 422. The read side is fail-honest. The toggle and username are plain columns and are compared, so a UI flip is caught. The password is gated behind a sensitive-data-enabled token at 4.1.2 and read:sensitive on v4.2, and would have to be printed as a field diff, so it is never projected into the comparison vocabulary on any box — every diff of an app declaring basic_auth says the password was NOT compared, in the backup schedule's voice: reported, not drift. custom_labels stays deliberately unwired: enabling basic auth or changing domains regenerates labels and overwrites it unless is_container_label_readonly_enabled, which is not API-settable until v4.2. The NO_API_COVERAGE row narrows to services, where it is a real API gap on both releases, plus a separate row for custom_labels on applications. Closes #76 Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 80 +++++++++ README.md | 19 ++- docs/semantics.md | 106 +++++++++++- src/apply.ts | 40 ++++- src/cli.ts | 104 +++++++++++- src/diff.ts | 98 +++++++++++ src/draft.ts | 39 ++++- src/manifest.ts | 76 +++++++++ src/resolve.ts | 96 ++++++++++- test/apply.test.ts | 101 ++++++++++++ test/basic-auth-cli.test.ts | 319 ++++++++++++++++++++++++++++++++++++ test/diff.test.ts | 247 ++++++++++++++++++++++++++++ test/manifest.test.ts | 118 +++++++++++++ test/wire.test.ts | 94 +++++++++++ 14 files changed, 1523 insertions(+), 14 deletions(-) create mode 100644 test/basic-auth-cli.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8929dce..c07e292 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,72 @@ actually cutting it, and this file starts there. ## Unreleased +### Added + +- **An application can declare HTTP basic auth, and `apply` sets it** + (#76) — `UNCAPTURED.md` has said for as long as it has existed that + Basic Auth is "carried as raw container labels. cast's manifest has no + field for them, so a rebuilt resource is UNPROTECTED where the original + was not." The #72 audit found that half of that is a **cast vocabulary + gap, not a Coolify one**: `is_http_basic_auth_enabled`, + `http_basic_auth_username` and `http_basic_auth_password` are in both + the create and the PATCH allowlists at v4.1.2 + (`ApplicationsController.php:914`, `:2368`). So an application now + declares it: + + ```yaml + admin: + basic_auth: + enabled: true + username: ops + password: ${ADMIN_BASIC_AUTH_PROD} + ``` + + The password is a **store ref and only a store ref** — the schema + refuses a literal rather than warning about one, because a manifest is + a reviewed, committed artifact and a literal there is a live password + in git forever. It resolves out of the environment's age store through + the same mechanism every env-template `${REF}` uses, and a missing or + empty entry fails the run before anything is written. + + Managing it is **opt-in**, the `is_static` rule for the same reason one + notch sharper: an unconditional `is_http_basic_auth_enabled: false` + would make the first apply after this ships strip the protection off + every app somebody enabled by hand in the UI. Omit the block to say + nothing, `enabled: false` to assert it is off. Enabling without both + credentials is refused at parse time *and* at the wire — Coolify's own + rule (`:2446-2463`), enforced before the request rather than discovered + as a 422 halfway through a run. + + **The read side is fail-honest, because the three fields do not read + back alike.** The toggle and the username are plain columns and are + compared like anything else — somebody turning basic auth off in the UI + *is* caught, which is most of the value. The password is gated behind a + sensitive-data-enabled token at 4.1.2 and behind the `read:sensitive` + ability on v4.2 (#77), so whether it arrives depends on the token, the + route and the release — and it would have to be *printed*, since a + field diff renders as `field: → `. So cast never + projects it into the comparison vocabulary on any box, and every diff + of an app declaring `basic_auth:` prints `http_basic_auth_password NOT + compared — verify in the Coolify UI`. Same disposition as an + unverifiable backup schedule: reported, not counted as drift. A read + returning none of the three names all three on that line instead, and + claims nothing at all. + + The limit that follows is stated rather than hidden: rotating *only* + the password in the store produces no field diff and therefore no + write. It lands on the next apply that touches basic auth for any other + reason — Coolify requires both credentials on any write that enables + it, so cast completes the whole triple whenever it sends one. + + `custom_labels` is **deliberately still absent**, though it is equally + settable: enabling basic auth or changing domains makes Coolify + regenerate an application's labels and overwrite `custom_labels` unless + `is_container_label_readonly_enabled`, which is itself not API-settable + until v4.2. Declaring both on one app would have cast silently destroy + what it was told to write. Basic-auth-only is the safe slice until + then. + ### Changed - **`state:needs-human` no longer waits on the cron to become true** (#131) @@ -81,6 +147,20 @@ actually cutting it, and this file starts there. `blocker:unrequested`: `MISSING` (nobody reviewed) and `STALE` (everybody reviewed an older head). Fixtures 51 → 72. +- **The `NO_API_COVERAGE` row for Basic Auth now says *services*** (#76) + — the blanket row covered applications and services alike, and became + wrong for half of them the moment applications could say `basic_auth:`. + What survives is a real API gap rather than a vocabulary one: + `ServicesController` carries no basic-auth fields and no + `custom_labels`, on v4.1.2 or on the v4.2 train, so no manifest field + could ever set them. A separate row now covers `custom_labels` on + applications — writable, deliberately unwired, with the overwrite + caveat spelled out — so neither row implies cast can express something + it cannot. `inventory --emit-draft` also reports, per application, an + app whose basic auth is enabled on the box: it emits no `basic_auth:` + block, because the password cannot be read and a block a rebuild cannot + honour is exactly the failure `UNCAPTURED.md` exists to prevent. + ### Fixed - **A PR that deletes a shipped release heading is now CI-red** (#133, diff --git a/README.md b/README.md index 06b9359..d81f5fb 100644 --- a/README.md +++ b/README.md @@ -397,6 +397,19 @@ standing over nothing is worse than no guard, because it reads like one. > prints in a diff like any literal. Applications only. See > [semantics.md](docs/semantics.md) → *Derived domains*. +> **An application can declare HTTP basic auth, and its password is a `${REF}` +> like any other secret.** Write +> `basic_auth: { enabled: true, username: ops, password: ${ADMIN_PW_PROD} }` on +> the application, and `apply` sets it — closing the "a rebuilt resource comes +> back UNPROTECTED" hole for applications (services have no API for it at all, on +> 4.1.2 or v4.2). The schema **refuses a literal password**: a manifest is a +> committed file, so a literal there is a password in git forever. What cast +> cannot do is *verify* it — the password reads back only to a token with +> sensitive-data reads, so `diff` compares the toggle and the username (a UI flip +> is still caught) and says on every run that the password was not compared, +> rather than implying it matches. See [semantics.md](docs/semantics.md) → *HTTP +> Basic Auth on an application*. + An **`--override`**'s value is read from `$CAST_CAPTURE_`, never from the command line: argv is visible in `ps` to every process on the box. It exists for values that must not survive the copy — staging and prod sharing a Mailgun @@ -569,8 +582,10 @@ whose secrets were silently skipped looks complete and holds not one value. **2. Silent losses.** cast cannot express everything a Coolify holds: destinations (which Docker network a resource sits on — no API at all in 4.1.2), service hostnames (they live per-container on `service.applications[].fqdn`), -Basic Auth and custom Traefik labels, the *Include Source Commit in Build* -toggle, whole database kinds (a MySQL is invisible to cast's manifest), backup +Basic Auth on a *service* and custom Traefik labels anywhere (an application's +Basic Auth is expressible — see below — but its **password** is not readable, so +a draft reports it instead of emitting a block a rebuild could not honour), the +*Include Source Commit in Build* toggle, whole database kinds (a MySQL is invisible to cast's manifest), backup schedules, and anything else configured in the UI with no manifest field. A blueprint that omits these **without saying so** is worse than no blueprint, diff --git a/docs/semantics.md b/docs/semantics.md index 6a8b382..884a2ef 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -291,6 +291,105 @@ draft always loads) — they used to sit in its `NO_HOME` list of settings a rebuild silently dropped, and `is_static` was not even there, which is exactly how a rebuilt static site came back wrong. +## HTTP Basic Auth on an application (`basic_auth:`) + +An application can declare HTTP basic auth, and `apply` sets it: + +```yaml +admin: + source: { repo: acme/widget, branch: main } + build: { pack: nixpacks, base_directory: / } + domains: ["https://admin.widget.example.com"] + basic_auth: + enabled: true + username: ops + password: ${ADMIN_BASIC_AUTH_PROD} +``` + +This closes the hole `UNCAPTURED.md` used to describe as *"a rebuilt resource is +UNPROTECTED where the original was not"* — for **applications**. It stays open +for **services**, and that is an API gap rather than a vocabulary one: +`ServicesController` carries no basic-auth fields and no `custom_labels`, on +v4.1.2 or on the v4.2 train, so no manifest field could set them (cast#72 +finding 7). The `NO_API_COVERAGE` row now says *services* specifically. + +**The password is a `${REF}`, and only a `${REF}`.** The schema refuses a +literal — not discourages it. A manifest is a reviewed, committed artifact, so a +literal there is a live password in git forever, in the file everyone reads to +understand the system. The value lives in the environment's age store under that +name, resolved by the same mechanism every env-template `${REF}` uses, and a +missing or empty entry fails the run **before** anything is written, naming the +ref and the store. + +**Managing basic auth is opt-in**, exactly like `is_static`. An omitted +`basic_auth:` block says *nothing* about basic auth, and that is deliberate: an +unconditional `is_http_basic_auth_enabled: false` would make the first apply +after this ships **strip the protection off every application somebody enabled by +hand in the UI** whose manifest had not yet been migrated. So: `enabled: true` to +protect, `enabled: false` to actively assert it is off, omit to leave it alone. +Enabling requires both credentials and disabling forbids them — both are +parse-time refusals, and the first is also Coolify's own rule +(`ApplicationsController.php:2446-2463` @ v4.1.2 rejects an enable without them), +enforced again at the wire so no path can reach a mid-run 422. + +### What the diff can and cannot see + +The three fields do not read back alike, and the report says which is which +rather than averaging them into a single confident answer: + +| field | read back? | consequence | +| --- | --- | --- | +| `is_http_basic_auth_enabled` | ✅ a plain column | compared — a toggle flipped in the UI **is** caught | +| `http_basic_auth_username` | ✅ a plain column | compared — a changed username **is** caught | +| `http_basic_auth_password` | ❌ | never compared, always **written** on any basic-auth write | + +The password is gated behind a sensitive-data-enabled token at 4.1.2, and on +v4.2 behind the `read:sensitive` token ability (cast#72, #77) — so whether it +arrives depends on the token, the route *and* the release. cast therefore never +projects it into the comparison vocabulary at all, on any box: a field that means +different things on different instances is worse than a field that means one +thing everywhere. (It would also have to be *printed* — `renderDiff` renders +every field diff as `field: → ` — and cast prints no secret +anywhere. The field name is redacted in the renderer as a backstop; not +projecting it is the actual guarantee.) + +So `cast diff` prints, on every run against an application that declares +`basic_auth:`: + +``` +basic_auth on application admin declared, http_basic_auth_password NOT compared — verify in the Coolify UI +``` + +Same disposition as an unverifiable backup schedule or a declared +`destination_uuid`: reported, **not** counted as drift (an absence of evidence is +not evidence of drift, and a run that fails on it is a run operators learn to +force past). A read that returns *none* of the three — a Coolify or a token that +does not serve those columns — names all three on that line instead, and cast +claims nothing at all about that application's basic auth. + +**The honest limit that follows:** rotating *only* the password in the store +produces no field diff, therefore no PATCH. The rotation lands on the next apply +that writes basic auth for any other reason (the toggle or username drifting, or +a create) — because Coolify requires both credentials on any write that enables +basic auth, so cast completes the whole triple whenever it sends one of them. +Until then the diff says the password was not compared rather than implying it +matches. To force a rotation today, flip `enabled` off, apply, flip it back on, +apply — or set it in the UI. + +### Why `custom_labels` is deliberately absent + +`custom_labels` **is** API-settable at 4.1.2 (base64-validated, +`ApplicationsController.php:3836-3854`, in both allowlists), and cast still has +no field for it. Enabling basic auth or changing domains makes Coolify regenerate +an application's proxy labels via `generateLabelsApplication()`, which +**overwrites `custom_labels`** unless `is_container_label_readonly_enabled` — and +*that* flag is itself not API-settable until v4.2. A manifest that declared both +`custom_labels` and domains or basic auth on one application would therefore have +cast silently destroy the labels it was just told to write. Basic-auth-only is +the safe slice until v4.2; raw labels wait for a real use and for the readonly +flag (cast#76, #77). Hand-written labels on a live box stay reported per resource +in `UNCAPTURED.md`, never silently dropped. + ## Reserved env var names (`SOURCE_COMMIT`, `COOLIFY_*`) Coolify injects a set of values into an application's runtime environment @@ -1063,8 +1162,11 @@ the per-service `GET /services/{uuid}` that `diff`/`apply` have made since projection the diff's read-back uses, so a drafted service diffs clean once applied; a service whose GET is unreachable or unrecognized is reported per resource instead — where a one-project diff fails closed, a whole-instance sweep -reports and keeps going), Basic Auth / custom Traefik labels, -build and deploy command overrides, what a backup schedule cannot fully say +reports and keeps going), Basic Auth on a SERVICE and custom Traefik labels +anywhere (an application's basic auth **is** expressible since #76 — what a draft +still cannot carry is its PASSWORD, which no read returns, so an enabled app is +reported per resource rather than emitted as a `basic_auth:` block a rebuild +could not honour), build and deploy command overrides, what a backup schedule cannot fully say (the schedule itself **is captured** since #75 — the draft reads `GET /databases/{uuid}/backups`, the route `diff`/`apply` have used since #51, and a single enabled schedule becomes a real `backup: { frequency, retention }` diff --git a/src/apply.ts b/src/apply.ts index b7973ff..5302970 100644 --- a/src/apply.ts +++ b/src/apply.ts @@ -92,6 +92,41 @@ export function applyHostnameOverlay( }); } +// Put the whole basic-auth triple back into an UPDATE payload that carries only +// part of it. +// +// An update body is assembled from the field DIFFS — the fields that actually +// changed — and that is wrong for basic auth in both directions: +// +// - Coolify requires username AND password on any write that enables basic +// auth (ApplicationsController.php:2446-2463 @ v4.1.2). So a drift in the +// toggle alone, or in the username alone, would PATCH an enable with a +// missing credential and 422 mid-run. +// - The password is never read back (see projectLiveFields), so it never +// appears as a diff on its own. Sending it alongside every basic-auth write +// is what makes a store-side rotation land at all: it rides on the next +// apply that touches basic auth for any reason. +// +// What it deliberately does NOT do is manufacture a write. A run where nothing +// about basic auth drifted still sends nothing — this only completes a payload +// that was already going to be sent. So the honest limit stands and is printed +// on every diff: rotating ONLY the password in the store produces no field diff, +// therefore no PATCH, and `cast diff` says the password was not compared rather +// than implying it matches. +export function completeBasicAuth( + fields: Record, + spec: Desired | undefined, +): Record { + if (fields.is_http_basic_auth_enabled !== true) return fields; + const declared = spec?.fields ?? {}; + const completed = { ...fields }; + for (const k of ["http_basic_auth_username", "http_basic_auth_password"]) { + if (completed[k] === undefined && declared[k] !== undefined) + completed[k] = declared[k]; + } + return completed; +} + export async function applyPlan( report: DiffReport, desired: Desired[], @@ -181,8 +216,9 @@ export async function applyPlan( uuid = await exec.createResource(c); } else { uuid = c.uuid as string; - const fields = Object.fromEntries( - c.fieldDiffs.map((f) => [f.field, f.desired]), + const fields = completeBasicAuth( + Object.fromEntries(c.fieldDiffs.map((f) => [f.field, f.desired])), + spec, ); if (Object.keys(fields).length > 0) { await exec.updateFields(uuid, c.kind, fields); diff --git a/src/cli.ts b/src/cli.ts index dd280b7..dfa9069 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -391,6 +391,53 @@ export function projectLiveFields( ...(raw.is_static == null ? {} : { is_static: raw.is_static === true || raw.is_static === 1 }), + // Basic auth, read back as far as the read path will say (cast#76). + // + // `is_http_basic_auth_enabled` and `http_basic_auth_username` are ordinary + // `applications` columns and come back on this route; they are projected + // whenever they are actually THERE, and omitted when they are not, so a + // Coolify (or a token) that hides them produces "not compared" rather than + // a phantom `undefined`. Same three-way discipline as `is_static` above, + // and note that "absent" and "false" are different answers: a real `false` + // is projected and diffs normally, which is what catches somebody turning + // basic auth off in the UI. + // + // `http_basic_auth_password` is NEVER projected, whatever the read + // returned, and that is a deliberate policy rather than a limitation: + // + // - It is gated. `ApplicationsController@removeSensitiveData` hides it + // from a token without sensitive-data reads at 4.1.2, and on `next` + // the hiding moves to the model behind the `read:sensitive` ability + // (cast#72, #77) — so whether it arrives depends on the token AND the + // route AND the release, and a diff must not silently mean different + // things on different boxes. + // - Even where it DOES arrive, putting a plaintext password into + // `fields` puts it into the diff report, and cast prints no secret, + // anywhere. (renderDiff redacts the field name as a backstop; not + // projecting it is the actual guarantee.) + // + // The consequence is stated rather than hidden: `fetchLive` flags the app + // `basicAuthNotCompared`, computeDiff skips the password, and every diff of + // an app declaring basic auth prints a line saying the password was not + // compared. A store-side password rotation therefore needs an apply that + // has some other reason to write — see completeBasicAuth in apply.ts, and + // the caveat in semantics.md. + // The username rides on the toggle's readability rather than on its own + // presence: the two are plain columns on the same row, serialized (or + // hidden) together, so a readable toggle means the username was readable + // too — and a NULL one then means "no username is set", a real value worth + // diffing against, not an unreadable one. Projecting it as absent instead + // would turn "somebody cleared the username" into "cast could not look". + ...(raw.is_http_basic_auth_enabled == null + ? {} + : { + is_http_basic_auth_enabled: + raw.is_http_basic_auth_enabled === true || + raw.is_http_basic_auth_enabled === 1, + http_basic_auth_username: String( + raw.http_basic_auth_username ?? "", + ), + }), ...(raw.install_command ? { install_command: raw.install_command } : {}), ...(raw.build_command ? { build_command: raw.build_command } : {}), ...(raw.start_command ? { start_command: raw.start_command } : {}), @@ -659,6 +706,20 @@ export async function fetchLive( ...(kind === "application" && i.is_static == null ? { staticNotCompared: true } : {}), + // Why this application's basic auth could not be fully verified (cast#76). + // Set on EVERY application, not only the ones a manifest protects — + // computeDiff decides whether it is relevant, because only it knows what + // the desired side declared. The password half is unconditional at 4.1.2 + // (projectLiveFields never projects it, on purpose); the whole-block half + // fires when the read returned no toggle at all. + ...(kind === "application" + ? { + basicAuthNotCompared: + i.is_http_basic_auth_enabled == null + ? "this read returned no is_http_basic_auth_enabled for this application, so cast saw none of its basic-auth state — a Coolify or a token that does not serve these columns on GET /projects/{uuid}/{env}" + : "http_basic_auth_password is never read back: Coolify 4.1.2 hides it from a token without sensitive-data reads, v4.2 moves it behind the read:sensitive ability, and cast prints no secret — so the toggle and the username are compared and the password is written, not verified (cast#76)", + } + : {}), })); const live = [ ...map("application", env.applications), @@ -1175,7 +1236,17 @@ async function runProject( let secrets: Record; if (existsSync(store)) { secrets = decryptSecrets(store, keyFileFor(ctx.envName)); - } else if (requiredSecrets(checkout, ctx.envName).required.length > 0) { + } else if ( + // `manifestRefs` alongside `required` (cast#76): a `basic_auth.password` + // ref is read from the store exactly like a template's, but it is not a + // RequiredSecret (see requiredSecrets). Asking only about `required` would + // let a manifest whose one secret is a basic-auth password proceed on `{}` + // — straight into desiredFromManifest's refusal, with a worse message. + (() => { + const s = requiredSecrets(checkout, ctx.envName); + return s.required.length > 0 || s.manifestRefs.length > 0; + })() + ) { throw new Error( [ `no secret store for ${orgRepo} in ${ctx.envName}`, @@ -2751,6 +2822,37 @@ export function applicationApiFields( ): Record { const { port, healthcheck, domains, docker_compose_domains, ...rest } = fields; + // Coolify's presence rule, enforced at the wire (cast#76). PATCH + // /applications/{uuid} rejects an enable without both credentials + // (ApplicationsController.php:2446-2463 @ v4.1.2) and the create allowlist + // takes the same three keys (:914, :2368). + // + // The manifest schema already refuses a half-declared block, so this is the + // BELT, not the braces — and it is worth having because it guards the paths + // the schema cannot see: apply's own field completion, a hostname overlay, and + // any future caller assembling a payload by hand. Failing here costs one + // exception; failing at Coolify costs a 422 in the middle of a run that has + // already created a project, an environment and possibly a database. + if (rest.is_http_basic_auth_enabled === true) { + const missing = ( + ["http_basic_auth_username", "http_basic_auth_password"] as const + ).filter((k) => { + const v = rest[k]; + return typeof v !== "string" || v === ""; + }); + if (missing.length > 0) { + throw new Error( + [ + `refusing to enable HTTP basic auth without ${missing.join(" and ")}`, + "", + "Coolify requires a username AND a password whenever basic auth is enabled, and", + "would answer 422 mid-run. Half-configured basic auth protects nothing anyway:", + "declare both under the application's `basic_auth:` (the password as a ${REF}", + "held by the environment's age store), or set `basic_auth.enabled: false`.", + ].join("\n"), + ); + } + } return { // is_static/install_command/build_command/start_command ride through `rest` // unchanged: they are valid API params verbatim, accepted on both the create diff --git a/src/diff.ts b/src/diff.ts index 1bad5e4..f8abb92 100644 --- a/src/diff.ts +++ b/src/diff.ts @@ -79,6 +79,27 @@ export type Live = { // a create-time setting; a real boolean from a future Coolify (staticNotCompared // unset) is projected and diffed normally. staticNotCompared?: boolean; + // Why one or more of this application's basic-auth fields could not be read + // back this run — printed verbatim. Third of the same family as + // backupNotCompared and staticNotCompared, and the one whose absence would be + // most dangerous, because the field it hides is a protection. + // + // Which fields it covers is not fixed: it is exactly the BASIC_AUTH_FIELDS + // that are ABSENT from `fields` (projectLiveFields omits what it could not + // read). Two shapes occur at 4.1.2, and the reason string says which: + // + // - the password alone is unreadable — the usual case. `enabled` and + // `username` are ordinary columns and diff normally, so a UI flip of the + // toggle or a changed username IS still caught; only a store-side password + // rotation is invisible. + // - nothing is readable — a read path or a token that returns none of the + // three. Then all three are skipped, and cast claims nothing at all about + // this app's basic auth. + // + // As with the other two, leaving the fields merely absent from `fields` is NOT + // equivalent: computeDiff would diff `true` against `undefined` and report + // confident drift, and apply would rewrite a protection it never read. + basicAuthNotCompared?: string; }; export type FieldDiff = { field: string; @@ -161,9 +182,38 @@ export type DiffReport = { // against `clean` — but printed on every run that has any, because the whole // point is that the assumption goes on screen at the moment it is made. backupsNotCompared: { name: string; reason: string }[]; + // Applications whose declared `basic_auth:` cast could not fully verify this + // run, with the fields it had to skip. Same disposition as + // backupsNotCompared — a finding, printed always, NOT counted against `clean`: + // it is an absence of evidence, not evidence of drift, and a run that failed + // because a read failed is a run operators learn to force past. + basicAuthNotCompared: { name: string; fields: string[]; reason: string }[]; clean: boolean; }; +// The three Coolify fields an application's `basic_auth:` block becomes. Named +// once, here, because three separate places have to agree on the set: the +// not-compared skip below, the redaction in renderDiff, and apply's +// completeBasicAuth. +export const BASIC_AUTH_FIELDS = [ + "is_http_basic_auth_enabled", + "http_basic_auth_username", + "http_basic_auth_password", +] as const; + +// Field names whose VALUES never reach the terminal, on either side of a diff. +// +// renderDiff prints every field diff as `field: → `, so an +// ordinary field carrying a password would print it twice — into a scrollback +// buffer, and into the CI log of every run. That is the same rule cast already +// holds for env vars (`secret X differs`, never the value) and for capture's +// disposition table; this is it, extended to the first RESOURCE FIELD that is a +// secret. The diff still says the field changed — what is withheld is only what +// it changed from and to. +export const REDACTED_FIELDS: ReadonlySet = new Set([ + "http_basic_auth_password", +]); + export const NON_UPDATABLE: Record = { application: ["build_pack"], database: ["type", "version"], @@ -311,6 +361,7 @@ export function computeDiff( ): DiffReport { const changes: Change[] = []; const backupsNotCompared: { name: string; reason: string }[] = []; + const basicAuthNotCompared: DiffReport["basicAuthNotCompared"] = []; // is_static is unreadable on Coolify 4.1.2's read path (cast#68); warn once // per run when the degradation actually bites (a manifest declares `static:` // on an app whose live value cast could not read), not per application. @@ -350,8 +401,32 @@ export function computeDiff( backupsNotCompared.push({ name: d.name, reason: l.backupNotCompared }); } const skipBackup = l.backupNotCompared !== undefined; + // The basic-auth escape hatch, third sibling of the backup and is_static + // ones. A field is skipped when the live read could not supply it (it is + // absent from `l.fields`) AND the read said why (`basicAuthNotCompared`) — + // never merely because it is absent, which would silently swallow a real + // "this app has no basic auth" into "cast could not tell". + // + // Recorded per APPLICATION, once, with the fields it covers, and only when + // the desired side declares basic auth at all: an app whose manifest is + // silent about it must not produce a line about something it never asked + // for. + const skippedBasicAuth = + l.basicAuthNotCompared === undefined + ? [] + : BASIC_AUTH_FIELDS.filter( + (f) => f in d.fields && !(f in l.fields), + ).map(String); + if (skippedBasicAuth.length > 0) { + basicAuthNotCompared.push({ + name: d.name, + fields: skippedBasicAuth, + reason: l.basicAuthNotCompared as string, + }); + } const fieldDiffs: FieldDiff[] = Object.entries(d.fields) .filter(([field]) => !(skipBackup && field === "backup")) + .filter(([field]) => !skippedBasicAuth.includes(field)) .filter(([field]) => { // The unreadable-is_static escape hatch (cast#68), sibling to the // backup one above. is_static lives on the ApplicationSetting relation, @@ -404,6 +479,7 @@ export function computeDiff( reserved, placement, backupsNotCompared, + basicAuthNotCompared, // A split project is drift, and drift is not clean — the same disposition // as an orphan: reported, counted, and NOT repaired (apply moves nothing // between networks; see renderDiff). @@ -454,6 +530,16 @@ export function renderDiff(report: DiffReport): string { for (const c of report.changes) { lines.push(`${c.op} ${c.kind} ${c.name}`); for (const f of c.fieldDiffs) { + // A redacted field says THAT it changes and never what to or from — see + // REDACTED_FIELDS. `f.live` is undefined here whenever the read could not + // see it, which is the common case, so even the shape of the old value + // would be a claim cast cannot make. + if (REDACTED_FIELDS.has(f.field)) { + lines.push( + ` ${f.field}: differs — apply will set it (secret; value not printed)`, + ); + continue; + } lines.push( ` ${f.field}: ${JSON.stringify(f.live)} → ${JSON.stringify(f.desired)}${f.updatable ? "" : " [NOT UPDATABLE IN PLACE]"}`, ); @@ -516,6 +602,18 @@ export function renderDiff(report: DiffReport): string { ` (${b.reason})`, ); } + // The same honest fallback as the backup line above, for the field where + // silence is most expensive: a diff that said "clean" over an unreadable basic + // auth would be a tool reporting an admin panel as protected without having + // looked. It names the fields it skipped so the line distinguishes "only the + // password" (the routine 4.1.2 case, where the toggle and username ARE + // compared) from "all of it" (a read that told cast nothing). + for (const b of report.basicAuthNotCompared) { + lines.push( + `basic_auth on application ${b.name} declared, ${b.fields.join(", ")} NOT compared — verify in the Coolify UI`, + ` (${b.reason})`, + ); + } const { placement } = report; if (placement.split) { lines.push( diff --git a/src/draft.ts b/src/draft.ts index cbc4779..1d6e2ce 100644 --- a/src/draft.ts +++ b/src/draft.ts @@ -424,7 +424,10 @@ function applicationSpec( // for. Each one changes what the application IS, and each would be silently // absent from a rebuild. const NO_HOME: Array<[string, string]> = [ - ["custom_labels", "custom Traefik/Docker labels (Basic Auth lives here)"], + [ + "custom_labels", + "custom Traefik/Docker labels (basic auth has its own fields, and its own manifest block since cast#76 — these are the hand-written labels beside it)", + ], ["ports_mappings", "host port mappings"], ["pre_deployment_command", "a pre-deployment command"], ["post_deployment_command", "a post-deployment command"], @@ -439,6 +442,26 @@ function applicationSpec( flag(field, `${what} is set on the box. The manifest has no field for it.`); } + // Basic auth is EXPRESSIBLE now (cast#76) and still not CAPTURABLE, and those + // are different sentences. The manifest has a `basic_auth:` block, so a draft + // could emit the toggle and the username — but the password reads back only to + // a token with sensitive-data reads (4.1.2; `read:sensitive` on v4.2) and a + // draft that emitted `enabled: true` with a password cast never saw would + // refuse to apply, or worse, apply with the wrong one. + // + // So the draft emits NOTHING here and says so per application, which is the + // same disposition as a generated secret: the name is reported, the value is + // the operator's to supply. Not emitting a half-block is the point — an + // `enabled: true` a rebuild cannot honour is the failure this whole file exists + // to prevent, and an unprotected admin panel is the one that costs most. + if (r.raw.is_http_basic_auth_enabled) { + const user = r.raw.http_basic_auth_username; + flag( + "basic_auth", + `HTTP basic auth is ENABLED on this application${typeof user === "string" && user !== "" ? ` (username ${user})` : ""}, and its PASSWORD cannot be read back — so no \`basic_auth:\` block was written and a rebuilt application would be PUBLIC. Add one by hand: \`basic_auth: { enabled: true, username: …, password: \${REF} }\`, with the value in the environment's age store.`, + ); + } + // `port` is one number in a manifest and a comma-separated list on the wire. // The draft writes the first and says so — a rebuilt app exposing one of the // three ports it used to is the kind of difference that surfaces as a broken @@ -876,9 +899,19 @@ const NO_API_COVERAGE: Array<[string, string]> = [ "destinations", "Coolify 4.1.2 serves no destinations endpoint. A resource's `destination_id` comes back; the UUID that names it never does. Placement must be read from the UI (#21).", ], + // Narrowed to SERVICES (cast#76). Applications can now say `basic_auth:`, so + // the blanket row overstated the gap for half the resources it covered — and + // the half it still covers is a real API gap, not a cast vocabulary one: + // `ServicesController` has no basic-auth fields and no `custom_labels`, on + // v4.1.2 or on `next` (cast#72, finding 7). The row must not imply cast could + // express this for a service if only someone wrote the field. [ - "Basic Auth / custom Traefik labels", - "carried as raw container labels. cast's manifest has no field for them, so a rebuilt resource is UNPROTECTED where the original was not.", + "Basic Auth / custom Traefik labels on SERVICES", + "no API surface at all — Coolify's ServicesController carries neither basic-auth fields nor custom_labels, on 4.1.2 or on the v4.2 train, so no manifest field could set them. A service that was protected on the source box comes back UNPROTECTED and must be re-protected by hand. (Applications are a different story: they take `basic_auth:` in the manifest since cast#76 — what is uncapturable there is the PASSWORD, reported per application above.)", + ], + [ + "custom Traefik/Docker labels on applications", + "`custom_labels` IS writable at 4.1.2, but cast deliberately has no field for it: enabling basic auth or changing domains makes Coolify regenerate an application's labels and overwrite whatever was there, unless `is_container_label_readonly_enabled` — which is itself not API-settable until v4.2. Declaring both would be a footgun, so labels set by hand stay uncaptured and are reported per application above (cast#72, #76).", ], [ '"Include Source Commit in Build"', diff --git a/src/manifest.ts b/src/manifest.ts index 4ba4c49..b47019c 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -34,6 +34,74 @@ const repoDirectoryPath = (field: string) => `${field} must be an absolute path inside the repo checkout (Coolify 4.1.2 rejects the create otherwise) — write /apps/core, not apps/core; the checkout root is /`, ); +// A store REF — `${NAME}` and nothing else. The one syntax cast already uses for +// a secret, in env templates (envtemplate.ts), reused verbatim rather than +// invented a second time: the value lives in the environment's age store, keyed +// by NAME, and the manifest carries only the name. +// +// This is a REFUSAL, not a preference. `http_basic_auth_password` is the first +// secret cast writes that is a resource FIELD rather than an env var, and a +// manifest is a reviewed, committed artifact — a literal here is a password in +// git, permanently, in the file everyone reads to understand the system. There is +// no ergonomic case that outweighs that, so the schema makes the mistake +// unrepresentable rather than warning about it. +const STORE_REF = /^\$\{([A-Za-z_][A-Za-z0-9_]*)\}$/; + +// HTTP Basic Auth on an application, as Coolify 4.1.2 can actually set it: +// `is_http_basic_auth_enabled`, `http_basic_auth_username` and +// `http_basic_auth_password` are in the create allowlist +// (`ApplicationsController.php:914`) and the PATCH allowlist (`:2368`), and PATCH +// enforces username/password presence when enabling (`:2446-2463`). +// +// `enabled` is explicit rather than inferred from the block's presence, because +// the two halves of the vocabulary are not symmetric: `enabled: true` needs +// credentials, `enabled: false` must have none (a password ref standing over a +// disabled auth is dead config that reads like a guard). Spelling it out is also +// what lets the presence rule below fail with a message about the field the +// operator got wrong, instead of a union mismatch about two shapes. +// +// OMITTING the block leaves basic auth alone entirely — the `is_static` rule +// (see resolve.ts), for the same reason: emitting `is_http_basic_auth_enabled: +// false` on every application would make the first apply after this ships strip +// basic auth off every app protected by hand in the UI whose manifest has not yet +// been migrated. Protection removed, silently, by an upgrade. So: declare +// `enabled: true` to protect, `enabled: false` to actively assert it is off, omit +// to say nothing. +const BasicAuthSchema = z + .object({ + enabled: z.boolean(), + username: z.string().optional(), + password: z + .string() + .regex( + STORE_REF, + "basic_auth.password must be a store ref (${NAME}) whose value lives in the environment's age store — never a literal, which would be a password committed to git", + ) + .optional(), + }) + .strict() + .superRefine((auth, ctx) => { + // Coolify's own rule, enforced HERE so it fails in the file rather than as a + // bare 422 from a PATCH that has already half-applied a run + // (ApplicationsController.php:2446-2463 @ v4.1.2 requires both when + // enabling). Same reasoning as the checkout-path patterns above. + if (auth.enabled) { + for (const k of ["username", "password"] as const) + if (auth[k] === undefined || auth[k] === "") + ctx.addIssue({ + code: "custom", + message: `basic_auth.${k} is required when basic_auth.enabled is true (Coolify rejects the write otherwise, and half-protected basic auth protects nothing)`, + }); + } else { + for (const k of ["username", "password"] as const) + if (auth[k] !== undefined) + ctx.addIssue({ + code: "custom", + message: `basic_auth.${k} is not allowed when basic_auth.enabled is false — a credential declared for a disabled auth is dead config that reads like a guard`, + }); + } + }); + const AppSpecSchema = z .object({ source: z.object({ repo: z.string(), branch: z.string() }).strict(), @@ -62,6 +130,7 @@ const AppSpecSchema = z healthcheck: z.string().optional(), domains: z.array(z.string()).optional(), service_domains: z.record(z.array(z.string())).optional(), + basic_auth: BasicAuthSchema.optional(), env_template: z.string().optional(), }) .strict() @@ -189,6 +258,13 @@ const ManifestSchema = z }) .strict(); +// The NAME inside a `${NAME}` store ref, or undefined if this is not one. The +// single reader of STORE_REF outside the schema, so the syntax the manifest +// ACCEPTS and the syntax resolution UNDERSTANDS cannot drift apart. +export function storeRefName(value: string): string | undefined { + return STORE_REF.exec(value)?.[1]; +} + export type AppSpec = z.infer; export type DatabaseSpec = z.infer; export type ServiceSpec = z.infer; diff --git a/src/resolve.ts b/src/resolve.ts index da2c059..1186f69 100644 --- a/src/resolve.ts +++ b/src/resolve.ts @@ -13,8 +13,8 @@ import { templateRefs, templateResourceRefs, } from "./envtemplate.js"; -import type { EnvironmentSpec } from "./manifest.js"; -import { loadManifest } from "./manifest.js"; +import type { AppSpec, EnvironmentSpec } from "./manifest.js"; +import { loadManifest, storeRefName } from "./manifest.js"; import { type ReservedHit, assertNoReservedEnvNames, @@ -376,7 +376,11 @@ function assertDomainRefs( export function requiredSecrets( checkoutDir: string, envName: string, -): { required: RequiredSecret[]; generated: string[] } { +): { + required: RequiredSecret[]; + generated: string[]; + manifestRefs: string[]; +} { const manifest = loadManifest(join(checkoutDir, ".infra", "manifest.yaml")); const envSpec = manifest.environments[envName]; if (!envSpec) { @@ -452,7 +456,26 @@ export function requiredSecrets( ].join("\n"), ); } - return { required, generated }; + // Store refs the MANIFEST itself carries, as opposed to the ones its env + // templates carry. Today that is exactly `basic_auth.password`. + // + // Kept OUT of `required`, deliberately. A RequiredSecret is `{ref, resource, + // key}` where `key` is a live ENV VAR name — that triple is what `capture` + // reads the source box's value from — and a basic-auth password is not an env + // var on any resource. Putting it in `required` would have capture look for an + // env var named after a field, fail to find it, and refuse the whole run as + // "missing". It is returned separately so the one caller that asks a different + // question — "would anything at all be read from the store?", the gate on + // whether a missing store is fatal (cli.ts, #104) — gets the right answer for + // a manifest whose only secret is a basic-auth password. + const manifestRefs = Object.values(envSpec.applications).flatMap((app) => { + const ref = + app.basic_auth?.enabled && app.basic_auth.password + ? storeRefName(app.basic_auth.password) + : undefined; + return ref ? [ref] : []; + }); + return { required, generated, manifestRefs }; } // What the manifest declares for an environment, as names only — no secrets, no @@ -527,6 +550,65 @@ export function canonicalizeServiceDomains( ); } +// The Coolify fields an application's `basic_auth:` block becomes, with the +// password resolved out of the age store. +// +// Empty when the manifest declares nothing: managing basic auth is OPT-IN, the +// same rule as `is_static` (see the comment on that field below) and for a +// sharper reason — an unconditional `is_http_basic_auth_enabled: false` would +// have the first apply after this ships REMOVE the protection from every +// application somebody enabled by hand in the UI. A tool that silently +// unprotects an admin panel during a routine apply is worse than one that cannot +// protect it at all. +// +// The password is resolved HERE, at plan time, from the same store every +// `${REF}` in an env template resolves against — so a missing ref fails before +// anything is written, naming the ref and the store, rather than 422ing +// mid-apply or (worse) writing an empty password over a working one. +export function basicAuthFields( + envName: string, + appName: string, + app: Pick, + secrets: Record, +): Record { + const auth = app.basic_auth; + if (!auth) return {}; + if (!auth.enabled) return { is_http_basic_auth_enabled: false }; + // Both are guaranteed present by the schema's superRefine; the checks are + // repeated at the value level because THIS is where an empty store entry + // becomes an empty password, which the schema cannot see. + const ref = storeRefName(String(auth.password)); + if (ref === undefined) { + throw new Error( + `manifest environment ${envName}: application ${appName} basic_auth.password is not a store ref (\${NAME})`, + ); + } + const value = secrets[ref]; + if (value === undefined || value === "") { + throw new Error( + [ + `manifest environment ${envName}: application ${appName} declares basic_auth.password \${${ref}}, and the age store ${value === "" ? "holds an EMPTY value for it" : "does not hold it"}`, + "", + " the store is the environment's `secrets/..env.age` — the same one", + " every env-template ${REF} resolves against. Add the name to it (a store is a", + " KEY=value file, encrypted to the environment's age recipient) and re-run.", + "", + "Writing an empty password would enable basic auth on a public URL and protect", + "nothing, and Coolify would accept it — so cast refuses before it writes anything.", + ].join("\n"), + ); + } + return { + is_http_basic_auth_enabled: true, + http_basic_auth_username: auth.username, + // The PLAINTEXT, in the desired field bag — the only place it exists in this + // process besides the decrypted store. It is never printed: renderDiff + // redacts this field name by name (see REDACTED_FIELDS in diff.ts), which is + // the same contract every secret env var already has. + http_basic_auth_password: value, + }; +} + export function desiredFromManifest( checkoutDir: string, envName: string, @@ -644,6 +726,12 @@ export function desiredFromManifest( ? { start_command: app.build.start_command } : {}), }), + // Outside the pack branch: basic auth is a property of the APPLICATION + // (Coolify sets it on the app's proxy labels, not on anything the build + // pack decides), so it is equally declarable on a compose app and a + // nixpacks one. The three keys are already Coolify's own names, so + // applicationApiFields passes them through untranslated. + ...basicAuthFields(envName, name, app, secrets), }, env: resolveEnvFile(name, app.env_template), }); diff --git a/test/apply.test.ts b/test/apply.test.ts index fca2def..41a64cc 100644 --- a/test/apply.test.ts +++ b/test/apply.test.ts @@ -4,6 +4,7 @@ import { KIND_ORDER, applyHostnameOverlay, applyPlan, + completeBasicAuth, } from "../src/apply.js"; import { GENERATED_PLACEHOLDER } from "../src/capture.js"; import { type Desired, type Live, computeDiff } from "../src/diff.js"; @@ -539,3 +540,103 @@ describe("applyHostnameOverlay", () => { expect(out[0].fields.domains).toEqual(["http://plain.example.net"]); }); }); + +// cast#76. An update body is built from the fields that CHANGED, and basic auth +// cannot be written that way: Coolify requires both credentials on any write +// that enables it, and the password never shows up as a change because it is +// never read back. So the payload is completed from the declared spec — and +// only when a write was already happening. +describe("completeBasicAuth", () => { + const spec: Desired = { + kind: "application", + name: "admin", + fields: { + build_pack: "nixpacks", + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + http_basic_auth_password: "s3cret", + }, + }; + + it("fills in the credentials when only the toggle drifted", () => { + expect( + completeBasicAuth({ is_http_basic_auth_enabled: true }, spec), + ).toEqual({ + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + http_basic_auth_password: "s3cret", + }); + }); + + it("fills in the password when only the username drifted", () => { + expect( + completeBasicAuth( + { is_http_basic_auth_enabled: true, http_basic_auth_username: "ops" }, + spec, + ).http_basic_auth_password, + ).toBe("s3cret"); + }); + + it("leaves a payload that is not enabling basic auth completely alone", () => { + // The load-bearing half: this must not MANUFACTURE a write. A run where + // nothing about basic auth drifted sends nothing about basic auth. + expect(completeBasicAuth({ domains: ["https://a"] }, spec)).toEqual({ + domains: ["https://a"], + }); + }); + + it("adds no credentials to a disable", () => { + expect( + completeBasicAuth({ is_http_basic_auth_enabled: false }, spec), + ).toEqual({ is_http_basic_auth_enabled: false }); + }); + + it("does not invent values the spec does not carry", () => { + // Then applicationApiFields refuses at the wire — one clear error, rather + // than a request Coolify 422s halfway through a run. + expect( + completeBasicAuth({ is_http_basic_auth_enabled: true }, undefined), + ).toEqual({ is_http_basic_auth_enabled: true }); + }); +}); + +// The honest limit, asserted rather than described: a password rotated in the +// store with nothing else changed produces NO write, because there is no field +// diff to carry it. `cast diff` prints "NOT compared" on that run — the failure +// is visible, not silent — and this test exists so the day someone makes the +// password diffable, it goes red and they read the comment. +describe("applyPlan — a password-only rotation writes nothing (#76)", () => { + it("makes no call at all when the readable halves agree", async () => { + const { calls, exec } = recorder(); + const declared: Desired[] = [ + { + kind: "application", + name: "admin", + fields: { + build_pack: "nixpacks", + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + http_basic_auth_password: "the-NEW-password", + }, + }, + ]; + const live: Live[] = [ + { + kind: "application", + name: "admin", + uuid: "u1", + fields: { + build_pack: "nixpacks", + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + }, + basicAuthNotCompared: "password is never read back", + }, + ]; + const report = computeDiff(declared, live, "full"); + await applyPlan(report, declared, exec); + expect(calls).toEqual([]); + // …and the run says so, rather than reading as a verified match. + expect(report.basicAuthNotCompared).toHaveLength(1); + }); +}); diff --git a/test/basic-auth-cli.test.ts b/test/basic-auth-cli.test.ts new file mode 100644 index 0000000..ff072b3 --- /dev/null +++ b/test/basic-auth-cli.test.ts @@ -0,0 +1,319 @@ +import { execFileSync, spawn } from "node:child_process"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeAll, describe, expect, it } from "vitest"; + +// HTTP basic auth on an application, end to end: manifest -> the real binary -> +// what goes on the wire and what reaches the terminal (cast#76). +// +// The unit tests prove each half (the schema refuses a literal password; +// projectLiveFields never projects one; computeDiff skips what it could not +// read; completeBasicAuth puts the triple back into a partial payload). This +// proves they are wired to each other, and pins the two facts that only a real +// request can show: that the CREATE body carries all three keys, and that a +// PATCH triggered by a drifted TOGGLE still carries the credentials Coolify +// requires alongside it. +// +// BOUNDARY, stated because it matters: the Coolify here is a stub of this +// repo's own making. These tests prove what cast SENDS. They cannot prove that a +// real 4.1.2 accepts it, that label regeneration behaves as cast#72 read it, or +// that a sensitive-data token returns the password on any given route — every +// one of those is a claim about Coolify, sourced from reading Coolify, and none +// has been run against a live instance. + +let recipient: string; +let keyFile: string; + +beforeAll(() => { + const dir = mkdtempSync(join(tmpdir(), "cast-age-")); + keyFile = join(dir, "age.key"); + execFileSync("age-keygen", ["-o", keyFile], { stdio: "pipe" }); + recipient = execFileSync("age-keygen", ["-y", keyFile], { + encoding: "utf8", + }).trim(); +}); + +type Stub = { + url: string; + hits: string[]; + bodies: Record>; + close: () => Promise; +}; +const stubs: Stub[] = []; + +// `app` is the knob: what the live application row looks like, or `null` for an +// environment with nothing in it (so the plan is a create). Everything else on +// the row matches the manifest below, so anything the diff reports is basic auth +// and nothing else. +async function stubCoolify(app: Record | null): Promise { + const hits: string[] = []; + const bodies: Record> = {}; + const server = createServer((req, res) => { + const path = new URL(req.url ?? "", "http://x").pathname.replace( + "/api/v1", + "", + ); + hits.push(`${req.method} ${path}`); + let raw = ""; + req.on("data", (d) => { + raw += String(d); + }); + req.on("end", () => { + if (raw !== "") { + try { + bodies[`${req.method} ${path}`] = JSON.parse(raw); + } catch { + /* not JSON — not a body this test asks about */ + } + } + const json = (body: unknown) => { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify(body)); + }; + if (path === "/teams/current") return json({ id: 0, name: "Root Team" }); + if (path === "/servers") + return json([{ uuid: "s1", name: "shared-box" }]); + if (path === "/github-apps") + return json([{ uuid: "gh1", name: "hdb-coolify" }]); + if (path === "/projects" && req.method === "GET") + return json([{ uuid: "p1", name: "incubator" }]); + if (path === "/projects/p1/environments") + return json([{ name: "staging" }]); + if (path === "/projects/p1/staging") + return json({ applications: app === null ? [] : [app] }); + if (path === "/applications" && req.method === "GET") return json([]); + if (path === "/applications/private-github-app" && req.method === "POST") + return json({ uuid: "app-1" }); + if (path === "/applications/app-1" && req.method === "PATCH") + return json({ uuid: "app-1" }); + if (path === "/applications/app-1/envs") return json([]); + if (path === "/deploy") return json({}); + res.writeHead(404); + res.end("{}"); + }); + }); + await new Promise((r) => { + server.listen(0, "127.0.0.1", r); + }); + const stub: Stub = { + url: `http://127.0.0.1:${(server.address() as AddressInfo).port}`, + hits, + bodies, + close: () => + new Promise((r) => { + server.close(() => r()); + }), + }; + stubs.push(stub); + return stub; +} + +afterEach(async () => { + await Promise.all(stubs.splice(0).map((s) => s.close())); +}); + +// A live application row as Coolify's environment_details serializes one. Note +// what is NOT here on purpose: `http_basic_auth_password`. That is the whole +// read-side story — the column is hidden from an ordinary token at 4.1.2, and +// cast would not project it even if it arrived. +const liveApp = (over: Record = {}) => ({ + name: "admin", + uuid: "app-1", + git_repository: "heavy-duty/incubator", + git_branch: "main", + build_pack: "nixpacks", + base_directory: "/", + fqdn: "https://admin.example.com", + destination_id: 1, + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + ...over, +}); + +const PASSWORD = "correct-horse-battery-staple"; + +const MANIFEST = `project: incubator +environments: + staging: + applications: + admin: + source: { repo: heavy-duty/incubator, branch: main } + build: { pack: nixpacks, base_directory: / } + domains: ["https://admin.example.com"] + basic_auth: + enabled: true + username: ops + password: \${ADMIN_BASIC_AUTH} +`; + +function fixture(url: string, store = `ADMIN_BASIC_AUTH=${PASSWORD}\n`) { + const checkout = mkdtempSync(join(tmpdir(), "cast-co-")); + mkdirSync(join(checkout, ".infra", "env"), { recursive: true }); + writeFileSync(join(checkout, ".infra", "manifest.yaml"), MANIFEST); + + const state = mkdtempSync(join(tmpdir(), "cast-state-")); + mkdirSync(join(state, "secrets")); + writeFileSync( + join(state, ".coolify.env"), + `COOLIFY_BASE_URL="${url}"\nCOOLIFY_ACCESS_TOKEN="t"\n`, + ); + execFileSync("age", ["-r", recipient, "-o", "incubator.staging.env.age"], { + input: store, + cwd: join(state, "secrets"), + stdio: ["pipe", "pipe", "pipe"], + }); + writeFileSync( + join(state, "environments.yaml"), + [ + "environments:", + " staging:", + " server: shared-box", + " team: { id: 0, name: Root Team }", + "github_apps:", + " incubator: hdb-coolify", + "", + ].join("\n"), + ); + return { checkout, state }; +} + +function run( + verb: "diff" | "apply", + f: { checkout: string; state: string }, +): Promise<{ code: number; output: string }> { + return new Promise((resolve) => { + const child = spawn( + "node", + [ + "dist/cli.js", + verb, + "heavy-duty/incubator", + "--env", + "staging", + "--path", + f.checkout, + "--state", + f.state, + ], + { + stdio: ["ignore", "pipe", "pipe"], + env: { ...process.env, CAST_AGE_KEY_FILE_STAGING: keyFile }, + }, + ); + let output = ""; + child.stdout.on("data", (d) => { + output += String(d); + }); + child.stderr.on("data", (d) => { + output += String(d); + }); + child.on("close", (code) => resolve({ code: code ?? 0, output })); + }); +} + +describe("cast apply — basic auth reaches the wire (#76)", () => { + it("sends all three keys on the CREATE, with the password out of the age store", async () => { + const stub = await stubCoolify(null); + const r = await run("apply", fixture(stub.url)); + expect(r.code).toBe(0); + const body = stub.bodies["POST /applications/private-github-app"]; + expect(body.is_http_basic_auth_enabled).toBe(true); + expect(body.http_basic_auth_username).toBe("ops"); + // The value came from the encrypted store via the manifest's ${REF} — the + // manifest itself holds only the name. + expect(body.http_basic_auth_password).toBe(PASSWORD); + }); + + // The case an update body assembled from field diffs alone would get wrong: + // only the toggle drifted, and Coolify rejects an enable without credentials. + it("sends the credentials alongside a toggle-only PATCH", async () => { + const stub = await stubCoolify( + liveApp({ is_http_basic_auth_enabled: false }), + ); + const r = await run("apply", fixture(stub.url)); + expect(r.code).toBe(0); + const body = stub.bodies["PATCH /applications/app-1"]; + expect(body.is_http_basic_auth_enabled).toBe(true); + expect(body.http_basic_auth_username).toBe("ops"); + expect(body.http_basic_auth_password).toBe(PASSWORD); + }); + + // The other half of the same rule: no drift, no write. cast does not PATCH + // basic auth onto every apply just because it cannot verify the password. + it("writes nothing when the readable halves already agree", async () => { + const stub = await stubCoolify(liveApp()); + const r = await run("apply", fixture(stub.url)); + expect(r.code).toBe(0); + expect(stub.hits).not.toContain("PATCH /applications/app-1"); + }); + + it("refuses before touching Coolify when the store does not hold the ref", async () => { + const stub = await stubCoolify(null); + const r = await run("apply", fixture(stub.url, "SOMETHING_ELSE=x\n")); + expect(r.code).not.toBe(0); + expect(r.output).toContain("does not hold it"); + // Nothing was created on the way to finding out. + expect(stub.hits).not.toContain("POST /applications/private-github-app"); + }); +}); + +describe("cast diff — basic auth is honest about the password (#76)", () => { + it("says the password was NOT compared, on a run it still calls clean", async () => { + const r = await run("diff", fixture((await stubCoolify(liveApp())).url)); + expect(r.code).toBe(0); + expect(r.output).toContain( + "basic_auth on application admin declared, http_basic_auth_password NOT compared", + ); + expect(r.output).toMatch(/^clean$/m); + }); + + it("never prints the password", async () => { + const r = await run("diff", fixture((await stubCoolify(null)).url)); + expect(r.output).not.toContain(PASSWORD); + }); + + // The defect this feature closes, from the other direction: somebody turned + // basic auth off on the box. An unreadable password must not make that + // invisible. + it("reports drift when the toggle was flipped off in the UI", async () => { + const r = await run( + "diff", + fixture( + (await stubCoolify(liveApp({ is_http_basic_auth_enabled: false }))).url, + ), + ); + expect(r.code).toBe(1); + expect(r.output).toContain("is_http_basic_auth_enabled: false → true"); + }); + + it("reports drift when the username was changed on the box", async () => { + const r = await run( + "diff", + fixture( + (await stubCoolify(liveApp({ http_basic_auth_username: "someone" }))) + .url, + ), + ); + expect(r.code).toBe(1); + expect(r.output).toContain("http_basic_auth_username"); + }); + + // A Coolify (or a token) that serves none of these columns must produce + // neither a clean bill nor invented drift. + it("claims nothing at all when the read carried no basic-auth state", async () => { + const stub = await stubCoolify( + liveApp({ + is_http_basic_auth_enabled: undefined, + http_basic_auth_username: undefined, + }), + ); + const r = await run("diff", fixture(stub.url)); + expect(r.output).toContain( + "is_http_basic_auth_enabled, http_basic_auth_username, http_basic_auth_password NOT compared", + ); + expect(r.output).toMatch(/^clean$/m); + }); +}); diff --git a/test/diff.test.ts b/test/diff.test.ts index 4dc871b..cf2d3d0 100644 --- a/test/diff.test.ts +++ b/test/diff.test.ts @@ -690,3 +690,250 @@ describe("backup schedules", () => { expect(out).toMatch(/^clean$/m); }); }); + +// Basic auth, read-side (cast#76). The password is a field cast WRITES and +// cannot READ, and the three ways that could go wrong are all worse than saying +// so: reporting a false "no change", reporting drift cast has no evidence for, +// or printing the value it does have. +describe("computeDiff — basic auth is fail-honest about what it could read", () => { + const wantAuth = { + kind: "application" as const, + name: "admin", + fields: { + build_pack: "nixpacks", + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + http_basic_auth_password: "s3cret", + }, + }; + // What projectLiveFields produces at 4.1.2: the toggle and the username, never + // the password — plus the reason, which fetchLive attaches. + const liveApp = ( + fields: Record, + over: Record = {}, + ) => ({ + kind: "application" as const, + name: "admin", + uuid: "app-1", + fields: { build_pack: "nixpacks", ...fields }, + ...over, + }); + const PW_REASON = "http_basic_auth_password is never read back"; + + it("compares the toggle and the username, and skips only the password", () => { + const r = computeDiff( + [wantAuth], + [ + liveApp( + { is_http_basic_auth_enabled: true, http_basic_auth_username: "ops" }, + { basicAuthNotCompared: PW_REASON }, + ), + ], + "full", + ); + // The two readable halves agree, so there is no drift to report — and the + // password does NOT become a phantom change against `undefined`. + expect(r.changes).toEqual([]); + expect(r.basicAuthNotCompared).toEqual([ + { + name: "admin", + fields: ["http_basic_auth_password"], + reason: PW_REASON, + }, + ]); + }); + + // The case the whole feature exists for: somebody turned basic auth off in the + // UI. The password being unreadable must not make cast blind to that. + it("still catches a toggle flipped off on the box", () => { + const r = computeDiff( + [wantAuth], + [ + liveApp( + { + is_http_basic_auth_enabled: false, + http_basic_auth_username: "ops", + }, + { basicAuthNotCompared: PW_REASON }, + ), + ], + "full", + ); + expect(r.changes).toHaveLength(1); + expect(r.changes[0].fieldDiffs.map((f) => f.field)).toEqual([ + "is_http_basic_auth_enabled", + ]); + expect(r.clean).toBe(false); + }); + + it("catches a username changed on the box", () => { + const r = computeDiff( + [wantAuth], + [ + liveApp( + { + is_http_basic_auth_enabled: true, + http_basic_auth_username: "someone-else", + }, + { basicAuthNotCompared: PW_REASON }, + ), + ], + "full", + ); + expect(r.changes[0].fieldDiffs.map((f) => f.field)).toEqual([ + "http_basic_auth_username", + ]); + }); + + // The other shape: a read that returned none of it. Then cast claims nothing + // about any of the three — not "clean", not "drifted". + it("skips all three when the read returned no basic-auth state at all", () => { + const r = computeDiff( + [wantAuth], + [liveApp({}, { basicAuthNotCompared: "no toggle on this read" })], + "full", + ); + expect(r.changes).toEqual([]); + expect(r.basicAuthNotCompared[0].fields).toEqual([ + "is_http_basic_auth_enabled", + "http_basic_auth_username", + "http_basic_auth_password", + ]); + }); + + it("says so on screen, on a run it still calls clean", () => { + const out = renderDiff( + computeDiff( + [wantAuth], + [ + liveApp( + { + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + }, + { basicAuthNotCompared: PW_REASON }, + ), + ], + "full", + ), + ); + expect(out).toContain( + "basic_auth on application admin declared, http_basic_auth_password NOT compared — verify in the Coolify UI", + ); + // Absence of evidence, not evidence of drift — the backup precedent. + expect(out).toMatch(/^clean$/m); + }); + + it("says nothing about basic auth for an application that declares none", () => { + const out = renderDiff( + computeDiff( + [ + { + kind: "application" as const, + name: "admin", + fields: { build_pack: "nixpacks" }, + }, + ], + [liveApp({}, { basicAuthNotCompared: PW_REASON })], + "full", + ), + ); + expect(out).not.toContain("basic_auth"); + expect(out).toMatch(/^clean$/m); + }); + + // A live resource that reports the fields fine (a future Coolify, or a + // sensitive-token read path) must NOT be told it was uncompared. + it("reports nothing uncompared when the read supplied everything it needed", () => { + const r = computeDiff( + [ + { + kind: "application" as const, + name: "admin", + fields: { build_pack: "nixpacks", is_http_basic_auth_enabled: false }, + }, + ], + [ + liveApp( + { is_http_basic_auth_enabled: false }, + { basicAuthNotCompared: PW_REASON }, + ), + ], + "full", + ); + expect(r.basicAuthNotCompared).toEqual([]); + }); +}); + +// The password must not reach a terminal, on either side of the arrow, on any +// path — including a CREATE, where every desired field becomes a field diff. +describe("renderDiff — the basic-auth password is redacted (#76)", () => { + const create = () => + renderDiff( + computeDiff( + [ + { + kind: "application" as const, + name: "admin", + fields: { + build_pack: "nixpacks", + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + http_basic_auth_password: "s3cret-value", + }, + }, + ], + [], + "full", + ), + ); + + it("never prints the value, on a create plan", () => { + expect(create()).not.toContain("s3cret-value"); + }); + + it("still says the field is being set — redacted is not silent", () => { + expect(create()).toContain( + "http_basic_auth_password: differs — apply will set it (secret; value not printed)", + ); + }); + + it("prints the username in the clear — it is not a secret", () => { + expect(create()).toContain("http_basic_auth_username"); + expect(create()).toContain("ops"); + }); + + it("never prints the value on an update plan either", () => { + const out = renderDiff( + computeDiff( + [ + { + kind: "application" as const, + name: "admin", + fields: { + build_pack: "nixpacks", + http_basic_auth_password: "s3cret-value", + }, + }, + ], + // No basicAuthNotCompared: this live side CAN see the password (a future + // Coolify), so it is compared — and still not printed. + [ + { + kind: "application" as const, + name: "admin", + uuid: "app-1", + fields: { + build_pack: "nixpacks", + http_basic_auth_password: "the-old-one", + }, + }, + ], + "full", + ), + ); + expect(out).not.toContain("s3cret-value"); + expect(out).not.toContain("the-old-one"); + expect(out).toContain("http_basic_auth_password: differs"); + }); +}); diff --git a/test/manifest.test.ts b/test/manifest.test.ts index beb26cd..dfb500c 100644 --- a/test/manifest.test.ts +++ b/test/manifest.test.ts @@ -285,6 +285,124 @@ environments: }); }); +// HTTP basic auth on an application (cast#76). The schema carries three rules, +// and each is here because breaking it is expensive somewhere else: the password +// must be a store ref (a literal is a password in git, forever), enabling needs +// both credentials (Coolify 422s mid-run otherwise, and half-configured basic +// auth protects nothing), and a disabled block must carry none (a credential +// standing over a disabled auth reads like a guard and is not one). +describe("loadManifest — basic_auth (#76)", () => { + const app = (basicAuth: string) => ` +project: x +environments: + prod: + applications: + admin: + source: { repo: o/r, branch: main } + build: { pack: nixpacks, base_directory: / } + domains: ["https://admin.example.com"] + basic_auth: ${basicAuth} +`; + + it("accepts an enabled block whose password is a ${REF}", () => { + const m = loadManifest(`${FIX}manifest.yaml`, { + overrideText: app( + "{ enabled: true, username: ops, password: '${ADMIN_PW}' }", + ), + }); + expect(m.environments.prod.applications.admin.basic_auth).toEqual({ + enabled: true, + username: "ops", + password: "${ADMIN_PW}", + }); + }); + + it("accepts a bare `enabled: false` — the way to assert basic auth is OFF", () => { + const m = loadManifest(`${FIX}manifest.yaml`, { + overrideText: app("{ enabled: false }"), + }); + expect(m.environments.prod.applications.admin.basic_auth).toEqual({ + enabled: false, + }); + }); + + it("treats an omitted block as saying nothing at all", () => { + const m = loadManifest(`${FIX}manifest.yaml`, { + overrideText: ` +project: x +environments: + prod: + applications: + admin: + source: { repo: o/r, branch: main } + build: { pack: nixpacks, base_directory: / } + domains: ["https://admin.example.com"] +`, + }); + expect(m.environments.prod.applications.admin.basic_auth).toBeUndefined(); + }); + + // The non-negotiable. A literal here would be a live password in a reviewed, + // committed file — so it is unrepresentable, not discouraged. + it("REFUSES a literal password", () => { + expect(() => + loadManifest(`${FIX}manifest.yaml`, { + overrideText: app( + "{ enabled: true, username: ops, password: hunter2 }", + ), + }), + ).toThrow(/must be a store ref/); + }); + + it("refuses a password that is a ref with anything around it", () => { + expect(() => + loadManifest(`${FIX}manifest.yaml`, { + overrideText: app( + "{ enabled: true, username: ops, password: 'pre-${ADMIN_PW}' }", + ), + }), + ).toThrow(/must be a store ref/); + }); + + // Coolify's own presence rule, failing in the FILE rather than as a 422 from a + // PATCH in the middle of a run. + it("refuses enabling without a password", () => { + expect(() => + loadManifest(`${FIX}manifest.yaml`, { + overrideText: app("{ enabled: true, username: ops }"), + }), + ).toThrow( + /basic_auth.password is required when basic_auth.enabled is true/, + ); + }); + + it("refuses enabling without a username", () => { + expect(() => + loadManifest(`${FIX}manifest.yaml`, { + overrideText: app("{ enabled: true, password: '${ADMIN_PW}' }"), + }), + ).toThrow( + /basic_auth.username is required when basic_auth.enabled is true/, + ); + }); + + it("refuses credentials declared alongside `enabled: false`", () => { + expect(() => + loadManifest(`${FIX}manifest.yaml`, { + overrideText: app("{ enabled: false, username: ops }"), + }), + ).toThrow(/not allowed when basic_auth.enabled is false/); + }); + + it("refuses a block with no `enabled` at all — the toggle is never inferred", () => { + expect(() => + loadManifest(`${FIX}manifest.yaml`, { + overrideText: app("{ username: ops, password: '${ADMIN_PW}' }"), + }), + ).toThrow(/enabled/); + }); +}); + describe("loadBindings", () => { it("parses bindings", () => { const b = loadBindings(`${FIX}environments.yaml`); diff --git a/test/wire.test.ts b/test/wire.test.ts index dc043b8..02deeeb 100644 --- a/test/wire.test.ts +++ b/test/wire.test.ts @@ -46,6 +46,51 @@ describe("applicationApiFields", () => { }); }); +// cast#76. The three basic-auth keys are already Coolify's own names, so the +// wire layer's job here is not translation — it is the presence rule, enforced +// before the request rather than discovered as a 422 halfway through a run. +describe("applicationApiFields — basic auth (#76)", () => { + it("passes the three basic-auth keys through untranslated", () => { + const out = applicationApiFields({ + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + http_basic_auth_password: "s3cret", + }); + expect(out).toEqual({ + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + http_basic_auth_password: "s3cret", + }); + }); + + it("passes a bare disable through — no credentials needed to turn it off", () => { + expect(applicationApiFields({ is_http_basic_auth_enabled: false })).toEqual( + { + is_http_basic_auth_enabled: false, + }, + ); + }); + + for (const [what, fields] of [ + ["no password", { http_basic_auth_username: "ops" }], + ["no username", { http_basic_auth_password: "s3cret" }], + [ + "an empty password", + { + http_basic_auth_username: "ops", + http_basic_auth_password: "", + }, + ], + ["neither", {}], + ] as const) { + it(`refuses to enable basic auth with ${what}`, () => { + expect(() => + applicationApiFields({ is_http_basic_auth_enabled: true, ...fields }), + ).toThrow(/refusing to enable HTTP basic auth without/); + }); + } +}); + describe("databaseApiFields", () => { it("maps type+version to an image and drops the type/version keys", () => { const out = databaseApiFields({ type: "postgresql", version: "17" }); @@ -145,6 +190,55 @@ describe("projectLiveFields", () => { expect(out.healthcheck).toBe("/health"); }); + // cast#76 — the read side of basic auth, and the rule that the password never + // enters the comparison vocabulary at all. + it("projects the basic-auth toggle and username, and NEVER the password", () => { + const out = projectLiveFields("application", { + git_repository: "org/repo", + git_branch: "main", + build_pack: "nixpacks", + base_directory: "/", + fqdn: "https://admin.example.com", + is_http_basic_auth_enabled: true, + http_basic_auth_username: "ops", + // Even when the read DOES carry it — a sensitive-data-enabled token — it + // must not reach `fields`, because a field in `fields` is a field + // renderDiff prints. + http_basic_auth_password: "s3cret", + }); + expect(out.is_http_basic_auth_enabled).toBe(true); + expect(out.http_basic_auth_username).toBe("ops"); + expect(out).not.toHaveProperty("http_basic_auth_password"); + expect(JSON.stringify(out)).not.toContain("s3cret"); + }); + + it("projects a real `false` toggle — off is an answer, not an absence", () => { + const out = projectLiveFields("application", { + git_repository: "org/repo", + git_branch: "main", + build_pack: "nixpacks", + base_directory: "/", + fqdn: "https://admin.example.com", + is_http_basic_auth_enabled: 0, + }); + expect(out.is_http_basic_auth_enabled).toBe(false); + // No username on the row means no username IS set — a value to diff against, + // not a field cast failed to read. + expect(out.http_basic_auth_username).toBe(""); + }); + + it("omits both when the read carries no toggle at all", () => { + const out = projectLiveFields("application", { + git_repository: "org/repo", + git_branch: "main", + build_pack: "nixpacks", + base_directory: "/", + fqdn: "https://admin.example.com", + }); + expect(out).not.toHaveProperty("is_http_basic_auth_enabled"); + expect(out).not.toHaveProperty("http_basic_auth_username"); + }); + it("normalizes a live database's database_type to the manifest vocabulary", () => { const out = projectLiveFields("database", { database_type: "standalone-postgresql",