Refuse to boot without R2 credentials
`?? ""` on the access key and secret meant a deployment with no object storage configured started happily, and `POST /jobs` handed clients presigned URLs signed with an empty key. The first sign of trouble was a 403 from R2 - either in the client's uploader or, worse, inside a gateway that had downloaded R2's XML error document in place of a print. Nothing in that chain said "R2 is not configured". Reads both credentials through a `required()` that throws, naming the variable, mirroring auth/verifier.ts and auth/edge_secret.ts. The endpoint is read with `||` rather than `??`, because the committed template ships `R2_ENDPOINT=` empty and `??` only catches null - so an empty value was taken as a real endpoint and the account-id derivation never ran. That is the trap auth/edge_secret.ts documents, and it caught this line too, in the first draft of it. A test covers the derivation. Each boot-failure test fills every other variable so the one under test is the only thing missing; otherwise whichever variable is read first wins and the assertion passes for the wrong reason. e2e-edge.sh gains dummy values, since it boots the backend. It presigns nothing, so they only have to exist.
This commit is contained in:
@@ -170,3 +170,82 @@ describe("configuration", () => {
|
||||
expect((await grep("process\\.env\\.EDGE_JWT_SECRET")).trim().split("\n")).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Object storage is a credential like any other, and used to be the one that
|
||||
* failed quietly.
|
||||
*
|
||||
* `R2_ACCESS_KEY_ID` and `R2_SECRET_ACCESS_KEY` defaulted to `""`, so a
|
||||
* deployment that copied `.env.example` and filled in everything else
|
||||
* started happily. `POST /jobs` then handed a client a presigned URL signed
|
||||
* with an empty key, and the first symptom was a 403 from R2 - in the
|
||||
* client's uploader, or inside a gateway that had downloaded R2's XML error
|
||||
* document instead of a print. Nothing in that chain said "R2 is not
|
||||
* configured".
|
||||
*/
|
||||
describe("object storage configuration", () => {
|
||||
const importS3 = (env: Record<string, string>) =>
|
||||
Bun.spawn(["bun", "-e", "await import('./src/s3/index.ts')"], {
|
||||
cwd: import.meta.dir + "/..",
|
||||
env: { ...process.env, ...env },
|
||||
stderr: "pipe",
|
||||
stdout: "pipe",
|
||||
});
|
||||
|
||||
// Every other variable is filled in, so the one under test is the only
|
||||
// thing missing and the error has to name it. Without this the first
|
||||
// missing variable in read order wins and the assertion passes for the
|
||||
// wrong reason.
|
||||
const complete = {
|
||||
R2_ACCESS_KEY_ID: "key",
|
||||
R2_SECRET_ACCESS_KEY: "secret",
|
||||
R2_ACCOUNT_ID: "acct",
|
||||
};
|
||||
|
||||
test("fails at boot when a credential is empty, naming the variable", async () => {
|
||||
for (const name of ["R2_ACCESS_KEY_ID", "R2_SECRET_ACCESS_KEY"]) {
|
||||
const proc = importS3({ ...complete, [name]: "" });
|
||||
expect(await proc.exited).not.toBe(0);
|
||||
expect(await new Response(proc.stderr).text()).toContain(name);
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* An empty string, not just an absent variable - which is the case that
|
||||
* actually happens, because the committed template ships these keys with
|
||||
* no value. `??` would have missed every one of them.
|
||||
*/
|
||||
test("treats an empty credential as missing, not as a value", async () => {
|
||||
const proc = importS3({ ...complete, R2_ACCESS_KEY_ID: "" });
|
||||
expect(await proc.exited).not.toBe(0);
|
||||
expect(await new Response(proc.stderr).text()).toContain("R2_ACCESS_KEY_ID");
|
||||
});
|
||||
|
||||
test("refuses to start with no endpoint and no account id to derive one from", async () => {
|
||||
const proc = importS3({ ...complete, R2_ENDPOINT: "", R2_ACCOUNT_ID: "" });
|
||||
expect(await proc.exited).not.toBe(0);
|
||||
expect(await new Response(proc.stderr).text()).toContain("R2_ENDPOINT");
|
||||
});
|
||||
|
||||
test("starts with a complete configuration", async () => {
|
||||
// R2_ENDPOINT empty on purpose: it must be derived from the account id,
|
||||
// which is the `||`-not-`??` case that caught this module out.
|
||||
const proc = importS3({ ...complete, R2_ENDPOINT: "" });
|
||||
expect(await proc.exited).toBe(0);
|
||||
});
|
||||
|
||||
test("reads each credential in exactly one place", async () => {
|
||||
// So there is only ever one line to get right, the same rule
|
||||
// `EDGE_JWT_SECRET` is held to above.
|
||||
for (const name of ["R2_ACCESS_KEY_ID", "R2_SECRET_ACCESS_KEY"]) {
|
||||
const proc = Bun.spawn(["grep", "-rn", `process.env.${name}`, "src"], {
|
||||
cwd: import.meta.dir + "/..",
|
||||
stdout: "pipe",
|
||||
});
|
||||
const hits = (await new Response(proc.stdout).text()).trim();
|
||||
// Read through the `required()` helper, so the literal name appears
|
||||
// only where it is passed in - not as a `process.env.X` at all.
|
||||
expect(hits).toBe("");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user