nextcloud: keep onlyoffice's jwt_secret off every argv (issue 1831)
occ's config:app:set put the secret on the docker exec command line, so it sat in the docker CLI's argv, the container's php argv and docker's exec record. Apply jwt_secret through config:import on occ's stdin instead; the other three onlyoffice keys still go through config:app:set.
This commit is contained in:
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<string, string>, opts: { checkFails?: boolean } = {}) {
|
||||
const calls: string[][] = [];
|
||||
const impl = (args: string[]): string => {
|
||||
const inputs: Array<string | undefined> = [];
|
||||
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<string, string>, 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<string, string> = {
|
||||
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();
|
||||
|
||||
Reference in New Issue
Block a user