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",