diff --git a/modules/nextcloud/client.ts b/modules/nextcloud/client.ts index e2310704..a3d4bc3b 100644 --- a/modules/nextcloud/client.ts +++ b/modules/nextcloud/client.ts @@ -92,11 +92,20 @@ export class NextcloudClient { return new NextcloudClient(container, ocsUrl.replace(/\/$/, ""), adminUser, adminPassword); } - /** Run occ inside the container as the web user, returning its stdout, throwing its own message. */ - occ(args: string[]): string { + /** + * Run occ inside the container as the web user, returning its stdout, throwing its own message. + * With `opts.input` given, occ reads it from stdin (`docker exec -i`) instead of taking it on the + * command line — the only way a secret value reaches occ without sitting in the docker CLI's argv, + * the container's php argv and docker's exec record (novox/hq issue 1831). + */ + occ(args: string[], opts: { input?: string } = {}): string { + const dockerArgs = opts.input !== undefined + ? ["exec", "-i", "-u", "www-data", this.container, "php", "occ", ...args] + : ["exec", "-u", "www-data", this.container, "php", "occ", ...args]; try { - return execFileSync("docker", ["exec", "-u", "www-data", this.container, "php", "occ", ...args], { + return execFileSync("docker", dockerArgs, { encoding: "utf8", timeout: 60_000, + ...(opts.input !== undefined ? { input: opts.input } : {}), }).trim(); } catch (err: any) { const detail = String(err?.stderr ?? err?.stdout ?? err?.message ?? "").trim(); @@ -126,7 +135,12 @@ export class NextcloudClient { const changed: string[] = []; for (const [occKey, value] of desired) { if (this.currentAppConfig(occKey) !== value) { - this.occ(["config:app:set", "onlyoffice", occKey, `--value=${value}`]); + // jwt_secret is a secret: apply it by stdin of config:import, never on an argv (issue 1831). + if (occKey === "jwt_secret") { + this.occ(["config:import"], { input: JSON.stringify({ apps: { onlyoffice: { jwt_secret: value } } }) }); + } else { + this.occ(["config:app:set", "onlyoffice", occKey, `--value=${value}`]); + } changed.push(occKey); } } diff --git a/modules/nextcloud/test/onlyoffice.test.ts b/modules/nextcloud/test/onlyoffice.test.ts index 09241597..52d4896d 100644 --- a/modules/nextcloud/test/onlyoffice.test.ts +++ b/modules/nextcloud/test/onlyoffice.test.ts @@ -33,8 +33,10 @@ function client(): any { /** A stand-in occ: config:app:get answers `current`, or throws like the real occ when unset. */ function standInOcc(current: Record, opts: { checkFails?: boolean } = {}) { const calls: string[][] = []; - const impl = (args: string[]): string => { + const inputs: Array = []; + const impl = (args: string[], occOpts: { input?: string } = {}): string => { calls.push(args); + inputs.push(occOpts.input); if (args[0] === "config:app:get") { const key = args[2]; if (!(key in current)) throw new Error("occ produced no output"); @@ -46,13 +48,18 @@ function standInOcc(current: Record, opts: { checkFails?: boolea current[key] = value; return "System config value onlyoffice => " + key + " set to string " + value; } + if (args[0] === "config:import") { + const parsed = JSON.parse(occOpts.input ?? "{}"); + Object.assign(current, parsed?.apps?.onlyoffice ?? {}); + return ""; + } if (args[0] === "onlyoffice:documentserver") { if (opts.checkFails) throw new Error("occ produced no output"); return ""; } throw new Error("unexpected occ call: " + args.join(" ")); }; - return { calls, impl }; + return { calls, inputs, impl }; } // 1830/AC3a: when the delivered values differ from Nextcloud's settings, it sets exactly the @@ -77,6 +84,32 @@ test("sets exactly the keys that differ, and runs the connection check", () => { assert.equal(current.StorageUrl, FULL_CONFIG.onlyofficeStorageUrl, "the matching key was left alone"); }); +// 1831/AC1: the jwt_secret never reaches a docker/occ argv — it is applied only as stdin of a +// config:import call, never as a config:app:set --value on the command line (novox/hq issue 1831). +test("the jwt_secret is set by stdin of config:import, never in an argv", () => { + const c = client(); + const current: Record = { + DocumentServerUrl: "https://old-office.example/", + DocumentServerInternalUrl: FULL_CONFIG.onlyofficeInternalUrl, + StorageUrl: FULL_CONFIG.onlyofficeStorageUrl, + jwt_secret: "a-stale-secret", + }; + const stand = standInOcc(current); + mock.method(c, "occ", stand.impl); + + const result = c.applyOnlyOffice(configFile(FULL_CONFIG)); + + assert.ok(result.changed.includes("jwt_secret"), "jwt_secret was changed"); + assert.ok(!stand.calls.some((a) => a.join(" ").includes(SECRET)), "the secret is in no argv"); + const importCall = stand.calls.findIndex((a) => a[0] === "config:import"); + assert.notEqual(importCall, -1, "a config:import call was made"); + assert.ok(!stand.calls.some((a) => a[0] === "config:app:set" && a[2] === "jwt_secret"), + "jwt_secret is never set via config:app:set"); + assert.equal(stand.inputs[importCall], JSON.stringify({ apps: { onlyoffice: { jwt_secret: SECRET } } }), + "the secret is passed only as stdin of config:import"); + assert.equal(current.jwt_secret, SECRET, "the secret was actually applied"); +}); + // 1830/AC3b: when they match, it changes nothing and runs no check. test("changes nothing and runs no check when every value already matches", () => { const c = client();