diff --git a/modules/minio/client.ts b/modules/minio/client.ts index 9ffb5b23..710962d3 100644 --- a/modules/minio/client.ts +++ b/modules/minio/client.ts @@ -228,9 +228,14 @@ export class MinioClient { * but the machine's process table shows it to every local user for the second `mc` runs (novox/hq * issue 282, open): replacing it means the admin API's encrypted request made here. */ - async createAccessKey(bucket: string, accessKey: string, secretKey: string): Promise { + async createAccessKey( + bucket: string, + accessKey: string, + secretKey: string, + policy: string = bucketPolicy(bucket), + ): Promise { const policyPath = join(this.mcConfigDir, `policy-${accessKey}.json`); - writeFileSync(policyPath, bucketPolicy(bucket), { mode: 0o600 }); + writeFileSync(policyPath, policy, { mode: 0o600 }); try { await this.mc( "admin", "user", "svcacct", "add", "mesh", this.rootUser, @@ -248,6 +253,16 @@ export class MinioClient { await this.mc("admin", "user", "svcacct", "rm", "mesh", accessKey); } + /** Whether a service account of this access key exists (`mc admin user svcacct info` fails when it does not). */ + async accessKeyExists(accessKey: string): Promise { + try { + await this.mc("admin", "user", "svcacct", "info", "mesh", accessKey); + return true; + } catch { + return false; + } + } + /** * Run `mc` against the alias `mesh`, which it is given as MC_HOST_mesh in its environment — the * root user and password inside the URL — rather than by an `mc alias set` that carried the root @@ -320,7 +335,7 @@ export class MinioClient { // is a line of this module's manifest, filled per consumer and delivered to both ends). Both // survived with no callers, which is the state a rule comes back from; they are gone. -function bucketPolicy(bucket: string): string { +export function bucketPolicy(bucket: string): string { return JSON.stringify({ Version: "2012-10-17", Statement: [{ diff --git a/modules/minio/links.ts b/modules/minio/links.ts new file mode 100644 index 00000000..b870f0df --- /dev/null +++ b/modules/minio/links.ts @@ -0,0 +1,64 @@ +// The link key — a second service account per bucket, beside the consumer's own key (novox/hq ADR 0468 §8, +// issue 1594). The object-store seat signs the links (issue 1516) with this key and never with the root +// account: its policy allows only s3:GetObject and s3:PutObject on the bucket's objects, so a link can read or +// write an object of that bucket and do nothing else. Its secret is minio's own: a file under the state +// directory (links/.secret, 0600), never in a grant, never on the bus, never in an emitted event. +// +// This file imports nothing of the SDK, so its tests run without the toolchain's packages. + +import { createHash, randomBytes } from "node:crypto"; +import { chmodSync, existsSync, mkdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +/** What the link key needs of the admin client (MinioClient has all three). */ +export interface LinkAdmin { + createAccessKey(bucket: string, accessKey: string, secretKey: string, policy?: string): Promise; + removeAccessKey(accessKey: string): Promise; + accessKeyExists(accessKey: string): Promise; +} + +/** The access key id of a bucket's link key: "link" and the first 16 hex digits of the SHA-256 of the bucket + * name (20 characters, the longest an access key may be; a bucket name may be 63). The signer of issue 1516 + * derives the same id. */ +export function linkKeyId(bucket: string): string { + return "link" + createHash("sha256").update(bucket, "utf8").digest("hex").slice(0, 16); +} + +/** Only get and put of the bucket's objects: no list, no delete, no bucket-level action. */ +export function linkPolicy(bucket: string): string { + return JSON.stringify({ + Version: "2012-10-17", + Statement: [{ + Effect: "Allow", + Action: ["s3:GetObject", "s3:PutObject"], + Resource: [`arn:aws:s3:::${bucket}/*`], + }], + }); +} + +export function linkSecretPath(dir: string, bucket: string): string { + return join(dir, `${bucket}.secret`); +} + +/** Make the bucket's link key if it is not there: the secret is kept if its file exists (so a key lost in + * minio is made again under the same secret), made fresh otherwise. Idempotent: a key that exists is left. */ +export async function ensureLinkKey(admin: LinkAdmin, dir: string, bucket: string): Promise { + const id = linkKeyId(bucket); + const path = linkSecretPath(dir, bucket); + mkdirSync(dir, { recursive: true, mode: 0o700 }); + let secret = existsSync(path) ? readFileSync(path, "utf8").trim() : ""; + if (secret !== "" && (await admin.accessKeyExists(id))) return; + if (secret === "") { + secret = randomBytes(32).toString("hex"); + writeFileSync(path, secret + "\n", { mode: 0o600 }); + chmodSync(path, 0o600); + } + try { await admin.removeAccessKey(id); } catch { /* none yet */ } + await admin.createAccessKey(bucket, id, secret, linkPolicy(bucket)); +} + +/** Revoke the bucket's link key; the bucket and the secret file's absence are left as they are. */ +export async function revokeLinkKey(admin: LinkAdmin, dir: string, bucket: string): Promise { + try { await admin.removeAccessKey(linkKeyId(bucket)); } catch { /* already gone */ } + try { unlinkSync(linkSecretPath(dir, bucket)); } catch { /* none */ } +} diff --git a/modules/minio/module.json b/modules/minio/module.json index 85b82530..72614a58 100644 --- a/modules/minio/module.json +++ b/modules/minio/module.json @@ -173,6 +173,7 @@ "MESH_MINIO_ROOT_USER": "meshroot", "MESH_MINIO_ROOT_PASSWORD_FILE": "${dir:state}/root.secret", "MESH_MINIO_REGION": "eu-west", + "MESH_MINIO_LINKS": "${dir:state}/links", "MESH_MINIO_MC_BIN": "mcli", "MESH_MINIO_MC_CONFIG": "${dir:state}/mc", "MESH_RECEIVES": "${dir:grants}/mesh.json" diff --git a/modules/minio/package.json b/modules/minio/package.json index 892b0977..017e2af8 100644 --- a/modules/minio/package.json +++ b/modules/minio/package.json @@ -4,6 +4,9 @@ "description": "minio — S3-compatible object store. Its admin client, tools, provisioner and events live here (novox/hq ADR 0039).", "type": "module", "private": true, + "scripts": { + "test": "node --test --experimental-strip-types 'test/*.test.ts'" + }, "dependencies": { "@novox/mesh-sdk": "^0.1.7" }, diff --git a/modules/minio/provisioner/handlers.ts b/modules/minio/provisioner/handlers.ts new file mode 100644 index 00000000..70052810 --- /dev/null +++ b/modules/minio/provisioner/handlers.ts @@ -0,0 +1,78 @@ +// The s3-bucket provisioner's create, remove and holds, with what they touch handed in, so a unit test drives +// them with a fake admin client (the provisioner's index.ts runs them against minio). + +import type { Provision } from "@novox/mesh-sdk/provisioner"; +import { ensureLinkKey, revokeLinkKey, type LinkAdmin } from "../links.js"; + +export interface Admin extends LinkAdmin { + bucketExists(bucket: string): Promise; + createBucket(bucket: string): Promise; + canReachAs(bucket: string, accessKey: string, secretKey: string): Promise; + readonly baseUrl: string; +} + +export interface Deps { + minio: Admin; + linksDir: string; + announce(type: string, body: unknown): Promise; +} + +export function handlers({ minio, linksDir, announce }: Deps) { + return { + + async create(p: Provision): Promise { + const bucket = bucketNamed(p.derived); + const accessKeyId = p.as; + + if (!(await minio.bucketExists(bucket))) await minio.createBucket(bucket); + // Re-mint the scoped key idempotently: drop any prior one under this id, then add it back with + // the mesh's secret. + try { await minio.removeAccessKey(accessKeyId); } catch { /* none yet — first provision */ } + await minio.createAccessKey(bucket, accessKeyId, p.password); + await ensureLinkKey(minio, linksDir, bucket); + + await announce("bucket.created", { + bucket, + consumer: p.consumer ?? "", + accessKey: accessKeyId, + endpoint: minio.baseUrl, + }); + }, + + async remove(p: { as: string; derived: Readonly> }): Promise { + const bucket = bucketNamed(p.derived); + + // Revoking the key is what cuts the consumer's access, and the bucket is kept, empty or not + // (novox/hq issue 241: a withdrawal never destroys a consumer's data — on 2026-10-04 a misread grants file withdrew every consumer at once). A bucket is removed by a person, never by this loop. + try { await minio.removeAccessKey(p.as); } catch { /* already gone */ } + await revokeLinkKey(minio, linksDir, bucket); + console.error(`[minio] ${p.as} withdrawn: access key and link key revoked, bucket ${bucket} kept`); + + await announce("bucket.removed", { bucket, accessKey: p.as }); + }, + + // Asked every minute by the harness: whether the backend still holds this consumer exactly as + // the mesh gave it, so a login lost behind the provisioner's back is made again (novox/hq issue 120). + async holds(p: Provision): Promise { + return minio.canReachAs(bucketNamed(p.derived), p.as, p.password); + }, + }; +} + +/** The bucket the mesh derived for this consumer. + * + * Absent means this module is running against a control plane that does not fill `${consumer:…}` + * yet, or a manifest whose `serves` block lost the line. Both are the same mistake from here — + * nobody said which bucket — and both are said rather than guessed: a provisioner that fell back + * to deriving one would restore the second rule and hide the fault behind a bucket that happens + * to be right. */ +function bucketNamed(derived: Readonly>): string { + const bucket = derived.bucket; + if (typeof bucket !== "string" || bucket === "") { + throw new Error( + "the mesh did not say which bucket this consumer gets: minio's manifest must serve " + + "`bucket` under s3-bucket (novox/hq ADR 0202)", + ); + } + return bucket; +} diff --git a/modules/minio/provisioner/index.ts b/modules/minio/provisioner/index.ts index 15308b3d..345bee46 100644 --- a/modules/minio/provisioner/index.ts +++ b/modules/minio/provisioner/index.ts @@ -22,63 +22,28 @@ import { runProvisioner, type Provision } from "@novox/mesh-sdk/provisioner"; import { emit } from "@novox/mesh-sdk/events"; import { MinioClient } from "../client.js"; +import { ensureLinkKey } from "../links.js"; +import { handlers } from "./handlers.js"; const minio = MinioClient.fromEnv(); +// minio's own link keys (issue 1594): the secrets live under the state directory, nowhere else. +const linksDir = process.env.MESH_MINIO_LINKS ?? ""; -runProvisioner("s3-bucket", { - async create(p: Provision): Promise { - const bucket = bucketNamed(p.derived); - const accessKeyId = p.as; - - if (!(await minio.bucketExists(bucket))) await minio.createBucket(bucket); - // Re-mint the scoped key idempotently: drop any prior one under this id, then add it back with - // the mesh's secret. - try { await minio.removeAccessKey(accessKeyId); } catch { /* none yet — first provision */ } - await minio.createAccessKey(bucket, accessKeyId, p.password); - - await announce("bucket.created", { - bucket, - consumer: p.consumer ?? "", - accessKey: accessKeyId, - endpoint: minio.baseUrl, - }); - }, - - async remove(p: { as: string; derived: Readonly> }): Promise { - const bucket = bucketNamed(p.derived); - - // Revoking the key is what cuts the consumer's access, and the bucket is kept, empty or not - // (novox/hq issue 241: a withdrawal never destroys a consumer's data — on 2026-10-04 a misread grants file withdrew every consumer at once). A bucket is removed by a person, never by this loop. - try { await minio.removeAccessKey(p.as); } catch { /* already gone */ } - console.error(`[minio] ${p.as} withdrawn: access key revoked, bucket ${bucket} kept`); - - await announce("bucket.removed", { bucket, accessKey: p.as }); - }, - - // Asked every minute by the harness: whether the backend still holds this consumer exactly as - // the mesh gave it, so a login lost behind the provisioner's back is made again (novox/hq issue 120). - async holds(p: Provision): Promise { - return minio.canReachAs(bucketNamed(p.derived), p.as, p.password); - }, -}); - -/** The bucket the mesh derived for this consumer. - * - * Absent means this module is running against a control plane that does not fill `${consumer:…}` - * yet, or a manifest whose `serves` block lost the line. Both are the same mistake from here — - * nobody said which bucket — and both are said rather than guessed: a provisioner that fell back - * to deriving one would restore the second rule and hide the fault behind a bucket that happens - * to be right. */ -function bucketNamed(derived: Readonly>): string { - const bucket = derived.bucket; - if (typeof bucket !== "string" || bucket === "") { - throw new Error( - "the mesh did not say which bucket this consumer gets: minio's manifest must serve " + - "`bucket` under s3-bucket (novox/hq ADR 0202)", - ); +// At start every bucket that exists gets its link key too, those made before link keys were (best effort: +// a bucket that fails is said, the provisioner still serves). +void (async () => { + if (linksDir === "") { console.error("[minio] MESH_MINIO_LINKS is not set: no link keys made"); return; } + try { + for (const b of await minio.listBuckets()) { + try { await ensureLinkKey(minio, linksDir, b.name); } + catch (err) { console.error(`[minio] link key for ${b.name}: ${err}`); } + } + } catch (err) { + console.error(`[minio] could not list buckets for link keys: ${err}`); } - return bucket; -} +})(); + +runProvisioner("s3-bucket", handlers({ minio, linksDir, announce })); /** Emit best-effort: a broker hiccup is logged and dropped, never allowed to throw back and fail a * bucket that was made. */ diff --git a/modules/minio/test/links.test.ts b/modules/minio/test/links.test.ts new file mode 100644 index 00000000..68e3f6b9 --- /dev/null +++ b/modules/minio/test/links.test.ts @@ -0,0 +1,112 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { mkdtempSync, readFileSync, statSync, existsSync, readdirSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +// Issue 1594: a link key per bucket (novox/hq ADR 0468 §8). The fake admin below stands for minio's service +// accounts; the real client's policies come from client.ts's bucketPolicy, which the tests import. + +import { bucketPolicy } from "../client.ts"; +import { ensureLinkKey, linkKeyId, linkSecretPath } from "../links.ts"; +import { handlers } from "../provisioner/handlers.ts"; + +class FakeAdmin { + baseUrl = "http://127.0.0.1:9000"; + buckets = new Set(); + keys = new Map(); + created: string[] = []; + async bucketExists(b: string) { return this.buckets.has(b); } + async createBucket(b: string) { this.buckets.add(b); } + async canReachAs() { return true; } + async accessKeyExists(id: string) { return this.keys.has(id); } + async removeAccessKey(id: string) { + if (!this.keys.delete(id)) throw new Error("no such key"); + } + async createAccessKey(bucket: string, id: string, secret: string, policy: string = bucketPolicy(bucket)) { + this.keys.set(id, { bucket, secret, policy }); + this.created.push(id); + return { accessKey: id, secretKey: secret }; + } +} + +function setup() { + const minio = new FakeAdmin(); + const dir = join(mkdtempSync(join(process.env.TMPDIR ?? tmpdir(), "minio-links-")), "links"); + const events: Array<{ type: string; body: unknown }> = []; + const h = handlers({ minio, linksDir: dir, announce: async (type, body) => { events.push({ type, body }); } }); + return { minio, dir, events, h }; +} + +const provision = { as: "brother-key", password: "consumer-secret", consumer: "brother", derived: { bucket: "brother" } } as any; + +test("creating a consumer's bucket makes a link key allowed only get and put of the bucket's objects, its secret in links/.secret at 0600", async () => { + // 1594/AC1 + const { minio, dir, h } = setup(); + await h.create(provision); + const link = minio.keys.get(linkKeyId("brother")); + assert.ok(link, "a link key exists for the bucket"); + const policy = JSON.parse(link.policy); + assert.equal(policy.Statement.length, 1); + assert.deepEqual(policy.Statement[0].Action, ["s3:GetObject", "s3:PutObject"]); + assert.deepEqual(policy.Statement[0].Resource, ["arn:aws:s3:::brother/*"]); + const path = linkSecretPath(dir, "brother"); + assert.equal(statSync(path).mode & 0o777, 0o600); + assert.equal(readFileSync(path, "utf8").trim(), link.secret); + assert.ok(link.secret.length >= 32); +}); + +test("buckets that existed before get a link key at start, and a second start makes none twice", async () => { + // 1594/AC2 + const { minio, dir } = setup(); + for (const b of ["brother", "mesh-mail", "nextcloud", "invoicing"]) minio.buckets.add(b); + for (const b of minio.buckets) await ensureLinkKey(minio, dir, b); + assert.equal(minio.keys.size, 4); + const first = [...minio.created]; + const secret = readFileSync(linkSecretPath(dir, "nextcloud"), "utf8"); + for (const b of minio.buckets) await ensureLinkKey(minio, dir, b); + assert.deepEqual(minio.created, first, "no key made a second time"); + assert.equal(readFileSync(linkSecretPath(dir, "nextcloud"), "utf8"), secret, "the secret is kept"); +}); + +test("a link key lost in minio is made again under the kept secret", async () => { + // 1594/AC2 + const { minio, dir } = setup(); + await ensureLinkKey(minio, dir, "brother"); + const secret = minio.keys.get(linkKeyId("brother"))!.secret; + minio.keys.clear(); + await ensureLinkKey(minio, dir, "brother"); + assert.equal(minio.keys.get(linkKeyId("brother"))!.secret, secret); +}); + +test("removing a consumer revokes its bucket's link key and keeps the bucket", async () => { + // 1594/AC3 + const { minio, dir, h } = setup(); + await h.create(provision); + assert.ok(minio.keys.has(linkKeyId("brother"))); + await h.remove({ as: "brother-key", derived: { bucket: "brother" } }); + assert.ok(!minio.keys.has(linkKeyId("brother")), "link key revoked"); + assert.ok(!minio.keys.has("brother-key"), "consumer key revoked"); + assert.ok(minio.buckets.has("brother"), "bucket kept"); + assert.ok(!existsSync(linkSecretPath(dir, "brother")), "secret file removed"); +}); + +test("the consumer's own key keeps its s3:* policy, and the link key's secret is in no event", async () => { + // 1594/AC4 + const { minio, dir, events, h } = setup(); + await h.create(provision); + const own = minio.keys.get("brother-key")!; + assert.equal(own.secret, "consumer-secret"); + assert.equal(own.policy, bucketPolicy("brother")); + assert.deepEqual(JSON.parse(own.policy).Statement[0].Action, ["s3:*"]); + const linkSecret = minio.keys.get(linkKeyId("brother"))!.secret; + await h.remove({ as: "brother-key", derived: { bucket: "brother" } }); + assert.ok(events.length >= 2); + for (const e of events) assert.ok(!JSON.stringify(e).includes(linkSecret), `${e.type} carries no link secret`); + assert.deepEqual(readdirSync(dir), [], "nothing else is kept in the links directory"); +}); + +test("a link key id fits an access key (20 characters) whatever the bucket name", () => { + assert.equal(linkKeyId("a".repeat(63)).length, 20); + assert.notEqual(linkKeyId("brother"), linkKeyId("mesh-mail")); +}); diff --git a/modules/minio/tsconfig.json b/modules/minio/tsconfig.json index c2a8df00..f1e72155 100644 --- a/modules/minio/tsconfig.json +++ b/modules/minio/tsconfig.json @@ -10,7 +10,9 @@ }, "include": [ "client.ts", + "links.ts", "tools/index.ts", - "provisioner/index.ts" + "provisioner/index.ts", + "provisioner/handlers.ts" ] } \ No newline at end of file