Merge pull request #52 from claude-hdb/fix/compose-file-path

fix(manifest): refuse checkout paths that are not absolute (#49)
This commit is contained in:
Daniel Marin 2026-07-14 23:41:59 +01:00 committed by GitHub
commit 73a92254b4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 146 additions and 10 deletions

View file

@ -82,12 +82,26 @@ manifest needs to change.
```yaml
core:
source: { repo: acme/widget, branch: main }
build: { pack: dockercompose, base_directory: /, compose_file: docker-compose.yaml }
build: { pack: dockercompose, base_directory: /, compose_file: /docker-compose.yaml }
service_domains:
api: ["https://api.widget.example.com"]
env_template: core.prod.env.template
```
**Checkout paths are absolute.** `build.compose_file`,
`build.base_directory` and `build.publish_directory` are all paths *inside the
repo checkout*, and all three must begin with `/` — the manifest schema refuses
them otherwise. This is Coolify's own rule, not cast's taste: on create it
validates `docker_compose_location` against
`ValidationPatterns::FILE_PATH_PATTERN` and `base_directory`/`publish_directory`
against `DIRECTORY_PATH_PATTERN` (both anchored on a leading slash; only the
directory pattern also admits the bare `/` root), and a `docker-compose.yaml`
with no slash comes back as a bare 422 — *after* `apply` has already created
the project and the environment. cast refuses at parse time instead, on every
verb, at zero API cost. It refuses rather than normalizes: the manifest is the
artifact under review, so the value is fixed in the file, once, not repaired in
memory on every run.
Internally cast keeps the map vocabulary (`docker_compose_domains:
{service: string[]}`) all the way through `resolve.ts`/`apply.ts`/diffing;
only `cli.ts`'s wire-translation layer (`applicationApiFields`) flattens it to

View file

@ -2,15 +2,47 @@ import { readFileSync } from "node:fs";
import { parse } from "yaml";
import { z } from "zod";
// Coolify validates every checkout-relative path on create and 422s anything
// that is not absolute. Transcribed from coolify v4.1.2:
// `bootstrap/helpers/api.php::sharedDataApplications()` binds
// `base_directory`/`publish_directory` to `ValidationPatterns::directoryPathRules()`
// and `docker_compose_location` to `ValidationPatterns::filePathRules()`, and
// `app/Support/ValidationPatterns.php` defines the two patterns below. The only
// difference between them: a file path needs at least one character after the
// slash, a directory path may be the bare `/` (the checkout root).
const COOLIFY_FILE_PATH = /^\/[a-zA-Z0-9._/~@+-]+$/;
const COOLIFY_DIRECTORY_PATH = /^\/[a-zA-Z0-9._/~@+-]*$/;
// These are refinements, not normalizations, and must stay that way: cast does
// not quietly rewrite what the manifest says. A value that would 422 gets fixed
// in the file, in a commit, once — not repaired in memory on every run. And the
// check belongs here, at parse time, because it is a property of the manifest
// and of nothing else: by the time a create returns its bare 422, `apply` has
// already made the project and the environment, and the run is half-applied.
const composeFilePath = z
.string()
.regex(
COOLIFY_FILE_PATH,
"compose_file must be an absolute path inside the repo checkout (Coolify 4.1.2 rejects the create otherwise) — write /docker-compose.yaml, not docker-compose.yaml",
);
const repoDirectoryPath = (field: string) =>
z
.string()
.regex(
COOLIFY_DIRECTORY_PATH,
`${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 /`,
);
const AppSpecSchema = z
.object({
source: z.object({ repo: z.string(), branch: z.string() }).strict(),
build: z
.object({
pack: z.enum(["nixpacks", "static", "dockerfile", "dockercompose"]),
base_directory: z.string(),
publish_directory: z.string().optional(),
compose_file: z.string().optional(),
base_directory: repoDirectoryPath("base_directory"),
publish_directory: repoDirectoryPath("publish_directory").optional(),
compose_file: composeFilePath.optional(),
})
.strict(),
port: z.number().int().optional(),

View file

@ -251,7 +251,7 @@ environments:
applications:
core:
source: { repo: heavy-duty/incubator, branch: main }
build: { pack: dockercompose, base_directory: /, compose_file: docker-compose.yaml }
build: { pack: dockercompose, base_directory: /, compose_file: /docker-compose.yaml }
service_domains:
api: ["https://api.example.com"]
env_template: core.prod.env.template

View file

@ -55,7 +55,7 @@ environments:
applications:
core:
source: { repo: acme/widget, branch: main }
build: { pack: dockercompose, base_directory: /, compose_file: docker-compose.yaml }
build: { pack: dockercompose, base_directory: /, compose_file: /docker-compose.yaml }
service_domains:
api: ["https://api.example.com"]
env_template: core.prod.env.template
@ -63,7 +63,7 @@ environments:
});
const app = m.environments.prod.applications.core;
expect(app.build.pack).toBe("dockercompose");
expect(app.build.compose_file).toBe("docker-compose.yaml");
expect(app.build.compose_file).toBe("/docker-compose.yaml");
expect(app.service_domains).toEqual({ api: ["https://api.example.com"] });
expect(app.port).toBeUndefined();
expect(app.healthcheck).toBeUndefined();
@ -86,7 +86,11 @@ environments:
}),
).toThrow(/compose_file/);
});
it("rejects a dockercompose app with top-level domains", () => {
// Coolify 4.1.2 validates docker_compose_location against
// ValidationPatterns::FILE_PATH_PATTERN on create and 422s a path with no
// leading slash — after `apply` has already made the project and the
// environment. The manifest knows this before any API call, so it refuses.
it("rejects a compose_file with no leading slash, and names the fix", () => {
expect(() =>
loadManifest(`${FIX}manifest.yaml`, {
overrideText: `
@ -99,6 +103,92 @@ environments:
build: { pack: dockercompose, base_directory: /, compose_file: docker-compose.yaml }
service_domains:
api: ["https://api.example.com"]
`,
}),
).toThrow(/compose_file must be an absolute path.*\/docker-compose\.yaml/s);
});
it("rejects a compose_file that is the bare root (a directory, not a file)", () => {
expect(() =>
loadManifest(`${FIX}manifest.yaml`, {
overrideText: `
project: widget
environments:
prod:
applications:
core:
source: { repo: acme/widget, branch: main }
build: { pack: dockercompose, base_directory: /, compose_file: / }
service_domains:
api: ["https://api.example.com"]
`,
}),
).toThrow(/compose_file must be an absolute path/);
});
it("rejects a base_directory with no leading slash", () => {
expect(() =>
loadManifest(`${FIX}manifest.yaml`, {
overrideText: `
project: widget
environments:
prod:
applications:
core:
source: { repo: acme/widget, branch: main }
build: { pack: nixpacks, base_directory: apps/core }
domains: ["https://api.example.com"]
`,
}),
).toThrow(/base_directory must be an absolute path/);
});
it("rejects a publish_directory with no leading slash", () => {
expect(() =>
loadManifest(`${FIX}manifest.yaml`, {
overrideText: `
project: widget
environments:
prod:
applications:
core:
source: { repo: acme/widget, branch: main }
build: { pack: static, base_directory: /, publish_directory: dist }
domains: ["https://api.example.com"]
`,
}),
).toThrow(/publish_directory must be an absolute path/);
});
// Coolify's DIRECTORY_PATH_PATTERN admits the bare "/" where FILE_PATH_PATTERN
// does not — every manifest in the wild says `base_directory: /`, so a shared
// "absolute path" rule that rejected it would refuse every manifest cast has.
it("accepts / as base_directory and a nested absolute publish_directory", () => {
const m = loadManifest(`${FIX}manifest.yaml`, {
overrideText: `
project: widget
environments:
prod:
applications:
core:
source: { repo: acme/widget, branch: main }
build: { pack: static, base_directory: /, publish_directory: /apps/web/dist }
domains: ["https://api.example.com"]
`,
});
const app = m.environments.prod.applications.core;
expect(app.build.base_directory).toBe("/");
expect(app.build.publish_directory).toBe("/apps/web/dist");
});
it("rejects a dockercompose app with top-level domains", () => {
expect(() =>
loadManifest(`${FIX}manifest.yaml`, {
overrideText: `
project: widget
environments:
prod:
applications:
core:
source: { repo: acme/widget, branch: main }
build: { pack: dockercompose, base_directory: /, compose_file: /docker-compose.yaml }
service_domains:
api: ["https://api.example.com"]
domains: ["https://api.example.com"]
`,
}),

View file

@ -295,7 +295,7 @@ environments:
applications:
core:
source: { repo: acme/widget, branch: main }
build: { pack: dockercompose, base_directory: /, compose_file: docker-compose.yaml }
build: { pack: dockercompose, base_directory: /, compose_file: /docker-compose.yaml }
service_domains:
api: ["https://api.widget.example.com"]
env_template: core.prod.env.template
@ -315,7 +315,7 @@ environments:
git_branch: "main",
build_pack: "dockercompose",
base_directory: "/",
docker_compose_location: "docker-compose.yaml",
docker_compose_location: "/docker-compose.yaml",
docker_compose_domains: {
api: ["https://api.widget.example.com"],
},