Compare commits

...
Author SHA1 Message Date
jschoubben 0cf7b2344f Write a moved contributor's declared defaults into the provisioner client tests' stand-in mesh-config (issue 1780) 2026-10-21 21:48:59 +02:00
mesh-tools 3793175894 Merge pull request 'Make minio's link-key secrets 40 characters and keep every mc argument's secret out of its errors (issue 1766)' (#422) from fix/minio-link-key-length into main 2026-10-21 19:00:13 +00:00
jschoubben b70f246622 Merge remote-tracking branch 'origin/main' into fix/minio-link-key-length
mesh/merge-gate head b70f24662298 merged into trunk cdbdb4747474 — pass: builds minio, sent nowhere; no bus step; 1 wait(s) for a person; every machine com…
mesh/repo-check head b70f24662298 merged into trunk cdbdb4747474 — pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-…
mesh/delivery delivered
2026-10-21 20:52:18 +02:00
jschoubben 689e61081d Merge remote-tracking branch 'origin/main' into fix/minio-link-key-length
mesh/merge-gate head 689e61081df9 merged into trunk cdbdb4747474 — pass: builds minio, sent nowhere; no bus step; 1 wait(s) for a person; every machine com…
mesh/repo-check head 689e61081df9 merged into trunk cdbdb4747474 — fail: its merge-check.sh failed: mesh-controller: 1 problem(s) in 139 manifest(s): modul…
mesh/delivery superseded: a newer head of the same pull request
2026-10-21 20:22:24 +02:00
jschoubben c83f48b821 Make link-key secrets 40 characters and keep every mc argument's secret out of its errors (issue 1766) 2026-10-21 20:21:56 +02:00
6 changed files with 120 additions and 6 deletions
@@ -703,3 +703,23 @@ func TestNatsResticsFilesFillFromTheBackupPlacement(t *testing.T) {
b.assign(t, "restic", plan("# a\n/srv/a\n", "# a\nstate\n"))
eventually(t, "a module taken away refills backups.conf", func() bool { return e.read(t, backups) == head+"# a\n/srv/a\n" })
}
// defaults writes the defaults a module's manifest declares as mesh-config resolves them (source default), for a
// moved module whose templates read config values (issue 1780).
func (b *aBus) defaults(t *testing.T, module string, manifest []byte) {
t.Helper()
var m struct {
Config map[string]struct {
Default json.RawMessage `json:"default"`
} `json:"config"`
}
if err := json.Unmarshal(manifest, &m); err != nil {
t.Fatal(err)
}
for key, d := range m.Config {
if len(d.Default) > 0 && string(d.Default) != "null" {
b.put(t, ConfigBucket, node+"."+module+"."+key, fmt.Sprintf(`{"node":%q,"module":%q,"key":%q,"value":%s,`+
`"source":"default"}`, node, module, key, d.Default))
}
}
}
@@ -158,6 +158,7 @@ func TestNatsTheEnvironmentOfTheCataloguesModulesIsWhatTheOldFormComposed(t *tes
if !manifest.Moved || len(manifest.Items) == 0 {
t.Fatalf("%s is not a moved contributor of items", m)
}
b.defaults(t, m, read(m+"/module.json"))
texts := map[string]string{}
var planned []PlannedItem
for _, it := range manifest.Items {
@@ -63,6 +63,7 @@ func TestNatsTheSessionFilesFilledFromTheirContributorsItemsAreWhatTheOldFormCom
if !c.Moved || len(c.Shell) != 0 {
t.Fatalf("%s is not moved or still holds shell entries", m)
}
b.defaults(t, m, []byte(read(m+"/module.json")))
texts := map[string]string{}
var planned []PlannedItem
for _, it := range c.Items {
+36 -5
View File
@@ -256,11 +256,22 @@ export class MinioClient {
if (argv.some((a) => a.includes(this.rootPassword))) {
throw new Error("refused to run mc: its command line would carry the root password (novox/hq issue 282)");
}
const { stdout } = await execFileAsync(this.mcBin, argv, {
timeout: 30_000,
env: { ...process.env, MC_HOST_mesh: mcHost(this.baseUrl, this.rootUser, this.rootPassword) },
});
return stdout.trim();
const mcHostValue = mcHost(this.baseUrl, this.rootUser, this.rootPassword);
try {
const { stdout } = await execFileAsync(this.mcBin, argv, {
timeout: 30_000,
env: { ...process.env, MC_HOST_mesh: mcHostValue },
});
return stdout.trim();
} catch (err) {
// A fresh error with only the redacted message: execFile's own carries the whole command line, and
// its cmd, stdout and stderr properties would carry it on (issue 1766).
const known = [mcHostValue, this.rootPassword, encodeURIComponent(this.rootPassword), ...secretArgs(argv)];
const e = err as { stderr?: unknown; message?: string };
const reason = typeof e.stderr === "string" && e.stderr.trim() !== "" ? e.stderr.trim() : (e.message ?? String(err));
const shown = argv.filter((a) => a !== this.mcConfigDir).join(" ");
throw new Error(redactSecrets(`mc ${shown} failed: ${reason}`, known));
}
}
// --- SigV4 request plumbing ---------------------------------------------
@@ -360,6 +371,26 @@ function uriEncode(str: string, encodeSlash: boolean): string {
return out;
}
/** The values that follow --secret-key and --access-key in an argument list. */
function secretArgs(argv: string[]): string[] {
const out: string[] = [];
argv.forEach((a, i) => { if ((a === "--secret-key" || a === "--access-key") && i + 1 < argv.length) out.push(argv[i + 1]); });
return out;
}
/**
* The one redaction every `mc` failure goes through (issue 1766): the token after --secret-key or
* --access-key (as a separate word or after "="), any MC_HOST_ value, and each known secret, wherever they
* appear in the text, become "[redacted]".
*/
export function redactSecrets(text: string, known: string[] = []): string {
let out = text
.replace(/(--(?:secret|access)-key(?:=|\s+))('[^']*'|"[^"]*"|\S+)/g, "$1[redacted]")
.replace(/(MC_HOST_\w+=)\S+/g, "$1[redacted]");
for (const k of known) if (k !== "") out = out.split(k).join("[redacted]");
return out;
}
/**
* The MC_HOST_<alias> URL `mc` reads an alias from: the endpoint with the credentials in its userinfo,
* each percent-encoded so a reserved character in either cannot end it early.
+7 -1
View File
@@ -36,6 +36,9 @@ export function linkPolicy(bucket: string): string {
});
}
/** A link key's secret: 40 hex characters, minio's limit for a secret key (issue 1766). */
const LINK_SECRET = /^[0-9a-f]{40}$/;
export function linkSecretPath(dir: string, bucket: string): string {
return join(dir, `${bucket}.secret`);
}
@@ -47,9 +50,12 @@ export async function ensureLinkKey(admin: LinkAdmin, dir: string, bucket: strin
const path = linkSecretPath(dir, bucket);
mkdirSync(dir, { recursive: true, mode: 0o700 });
let secret = existsSync(path) ? readFileSync(path, "utf8").trim() : "";
// A stored secret of the wrong shape (an earlier version made 64 characters; minio takes at most 40) is
// replaced, and its key made again (issue 1766).
if (!LINK_SECRET.test(secret)) secret = "";
if (secret !== "" && (await admin.accessKeyExists(id))) return;
if (secret === "") {
secret = randomBytes(32).toString("hex");
secret = randomBytes(20).toString("hex");
writeFileSync(path, secret + "\n", { mode: 0o600 });
chmodSync(path, 0o600);
}
+55
View File
@@ -115,3 +115,58 @@ test("the link key id is the one the object-store tool derives (cmd/object-store
// 1516/AC1
assert.equal(linkKeyId("brother"), "link0b4bd77cec705cf5");
});
// Issue 1766: a link key's secret is minio's length, and no mc failure carries an argument's secret.
import { chmodSync, writeFileSync, mkdirSync } from "node:fs";
import { MinioClient } from "../client.ts";
test("a link key's secret is 40 characters, and a stored 64-character secret is replaced", async () => {
// 1766/AC1
const { minio, dir } = setup();
await ensureLinkKey(minio, dir, "fresh");
assert.equal(minio.keys.get(linkKeyId("fresh"))!.secret.length, 40);
assert.match(minio.keys.get(linkKeyId("fresh"))!.secret, /^[0-9a-f]{40}$/);
mkdirSync(dir, { recursive: true });
const old = "ab".repeat(32);
writeFileSync(linkSecretPath(dir, "old"), old + "\n");
await ensureLinkKey(minio, dir, "old");
const made = minio.keys.get(linkKeyId("old"))!.secret;
assert.equal(made.length, 40);
assert.notEqual(made, old);
assert.equal(readFileSync(linkSecretPath(dir, "old"), "utf8").trim(), made);
});
function failingMc(): { client: MinioClient; root: string } {
const d = mkdtempSync(join(process.env.TMPDIR ?? tmpdir(), "minio-mc-"));
const bin = join(d, "mc");
// Says its arguments and its alias on stderr, then fails, the way a refused command does.
writeFileSync(bin, '#!/bin/sh\necho "args: $*" >&2\necho "env: MC_HOST_mesh=$MC_HOST_mesh" >&2\nexit 1\n');
chmodSync(bin, 0o755);
const root = "RootPass-s3cret/ok";
const client = new MinioClient({ endpoint: "http://127.0.0.1:9000", rootUser: "root", rootPassword: root, region: "us-east-1", mcBin: bin, mcConfigDir: d });
return { client, root };
}
test("an mc failure's message holds no --secret-key value and no MC_HOST value", async () => {
// 1766/AC2
const { client, root } = failingMc();
const secret = "SuperSecretValue0123456789";
const err: any = await client.createAccessKey("b", "ACCESSKEYID1234567", secret).then(() => null, (e) => e);
assert.ok(err, "the failure is raised");
for (const text of [String(err.message), String(err.stack ?? ""), JSON.stringify(Object.getOwnPropertyNames(err).map((k) => err[k]))]) {
assert.ok(!text.includes(secret), "no secret key");
assert.ok(!text.includes("ACCESSKEYID1234567"), "no access key");
assert.ok(!text.includes(root) && !text.includes(encodeURIComponent(root)), "no root password");
assert.ok(!text.includes("root:"), "no MC_HOST value");
}
assert.match(err.message, /\[redacted\]/);
});
test("a consumer key's create passes the minted secret unchanged", async () => {
// 1766/AC3
const s = setup();
await s.h.create(provision);
assert.equal(s.minio.keys.get("brother-key")!.secret, "consumer-secret");
});