fix(manifest): refuse checkout paths that are not absolute (#49)
Coolify 4.1.2 validates every checkout-relative path on create and 422s anything without a leading slash: `docker_compose_location` against ValidationPatterns::FILE_PATH_PATTERN, `base_directory`/`publish_directory` against DIRECTORY_PATH_PATTERN. cast passed all three through verbatim and validated nothing about their shape — and docs/semantics.md taught the exact value Coolify rejects (`compose_file: docker-compose.yaml`). The 422 is a property of the manifest, not of the instance, so it is knowable before a single API call. Refine at parse time, on every verb: a refusal that names the fix, not a silent normalization — the manifest is the artifact under review, so a bad value is fixed in the file, once. Fixes the docs example and the fixtures that carried the broken value. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
7cc3a3b64e
commit
9a1b7003b7
5 changed files with 146 additions and 10 deletions
|
|
@ -82,12 +82,26 @@ manifest needs to change.
|
||||||
```yaml
|
```yaml
|
||||||
core:
|
core:
|
||||||
source: { repo: acme/widget, branch: main }
|
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:
|
service_domains:
|
||||||
api: ["https://api.widget.example.com"]
|
api: ["https://api.widget.example.com"]
|
||||||
env_template: core.prod.env.template
|
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:
|
Internally cast keeps the map vocabulary (`docker_compose_domains:
|
||||||
{service: string[]}`) all the way through `resolve.ts`/`apply.ts`/diffing;
|
{service: string[]}`) all the way through `resolve.ts`/`apply.ts`/diffing;
|
||||||
only `cli.ts`'s wire-translation layer (`applicationApiFields`) flattens it to
|
only `cli.ts`'s wire-translation layer (`applicationApiFields`) flattens it to
|
||||||
|
|
|
||||||
|
|
@ -2,15 +2,47 @@ import { readFileSync } from "node:fs";
|
||||||
import { parse } from "yaml";
|
import { parse } from "yaml";
|
||||||
import { z } from "zod";
|
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
|
const AppSpecSchema = z
|
||||||
.object({
|
.object({
|
||||||
source: z.object({ repo: z.string(), branch: z.string() }).strict(),
|
source: z.object({ repo: z.string(), branch: z.string() }).strict(),
|
||||||
build: z
|
build: z
|
||||||
.object({
|
.object({
|
||||||
pack: z.enum(["nixpacks", "static", "dockerfile", "dockercompose"]),
|
pack: z.enum(["nixpacks", "static", "dockerfile", "dockercompose"]),
|
||||||
base_directory: z.string(),
|
base_directory: repoDirectoryPath("base_directory"),
|
||||||
publish_directory: z.string().optional(),
|
publish_directory: repoDirectoryPath("publish_directory").optional(),
|
||||||
compose_file: z.string().optional(),
|
compose_file: composeFilePath.optional(),
|
||||||
})
|
})
|
||||||
.strict(),
|
.strict(),
|
||||||
port: z.number().int().optional(),
|
port: z.number().int().optional(),
|
||||||
|
|
|
||||||
|
|
@ -251,7 +251,7 @@ environments:
|
||||||
applications:
|
applications:
|
||||||
core:
|
core:
|
||||||
source: { repo: heavy-duty/incubator, branch: main }
|
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:
|
service_domains:
|
||||||
api: ["https://api.example.com"]
|
api: ["https://api.example.com"]
|
||||||
env_template: core.prod.env.template
|
env_template: core.prod.env.template
|
||||||
|
|
|
||||||
|
|
@ -55,7 +55,7 @@ environments:
|
||||||
applications:
|
applications:
|
||||||
core:
|
core:
|
||||||
source: { repo: acme/widget, branch: main }
|
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:
|
service_domains:
|
||||||
api: ["https://api.example.com"]
|
api: ["https://api.example.com"]
|
||||||
env_template: core.prod.env.template
|
env_template: core.prod.env.template
|
||||||
|
|
@ -63,7 +63,7 @@ environments:
|
||||||
});
|
});
|
||||||
const app = m.environments.prod.applications.core;
|
const app = m.environments.prod.applications.core;
|
||||||
expect(app.build.pack).toBe("dockercompose");
|
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.service_domains).toEqual({ api: ["https://api.example.com"] });
|
||||||
expect(app.port).toBeUndefined();
|
expect(app.port).toBeUndefined();
|
||||||
expect(app.healthcheck).toBeUndefined();
|
expect(app.healthcheck).toBeUndefined();
|
||||||
|
|
@ -86,7 +86,11 @@ environments:
|
||||||
}),
|
}),
|
||||||
).toThrow(/compose_file/);
|
).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(() =>
|
expect(() =>
|
||||||
loadManifest(`${FIX}manifest.yaml`, {
|
loadManifest(`${FIX}manifest.yaml`, {
|
||||||
overrideText: `
|
overrideText: `
|
||||||
|
|
@ -99,6 +103,92 @@ environments:
|
||||||
build: { pack: dockercompose, base_directory: /, compose_file: docker-compose.yaml }
|
build: { pack: dockercompose, base_directory: /, compose_file: docker-compose.yaml }
|
||||||
service_domains:
|
service_domains:
|
||||||
api: ["https://api.example.com"]
|
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"]
|
domains: ["https://api.example.com"]
|
||||||
`,
|
`,
|
||||||
}),
|
}),
|
||||||
|
|
|
||||||
|
|
@ -295,7 +295,7 @@ environments:
|
||||||
applications:
|
applications:
|
||||||
core:
|
core:
|
||||||
source: { repo: acme/widget, branch: main }
|
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:
|
service_domains:
|
||||||
api: ["https://api.widget.example.com"]
|
api: ["https://api.widget.example.com"]
|
||||||
env_template: core.prod.env.template
|
env_template: core.prod.env.template
|
||||||
|
|
@ -315,7 +315,7 @@ environments:
|
||||||
git_branch: "main",
|
git_branch: "main",
|
||||||
build_pack: "dockercompose",
|
build_pack: "dockercompose",
|
||||||
base_directory: "/",
|
base_directory: "/",
|
||||||
docker_compose_location: "docker-compose.yaml",
|
docker_compose_location: "/docker-compose.yaml",
|
||||||
docker_compose_domains: {
|
docker_compose_domains: {
|
||||||
api: ["https://api.widget.example.com"],
|
api: ["https://api.widget.example.com"],
|
||||||
},
|
},
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue