row 243: tests that depend on the gate umask set their file modes; stack-instances bootstraps under an explicit umask with a 077 subtest (#1754)
process.test, proof-cli.test and sso.test set the too-open mode with chmod instead of taking it from the umask. stack-instances.test bootstraps under an explicit umask and asserts the 0700 config dir under umask 077. Candidate morgana/row2436581f750e(code137bda3af), Webby APPROVE rev 2939.
This commit is contained in:
1 parent
9a8c8860d3
commit
6096b051b1
5 files changed
+147
-3
No files matched your search
@@ -0,0 +1,128 @@
|
||||
# Row 243: too-open mode refusal tests set their mode with chmod
|
||||
|
||||
Brief: `docs/plans/2026-10-21_gate-env-rows.md` § "The startup token refusal test sets its file mode"
|
||||
(Sage, sage-1909). Reviewer Webby. Lands after row 197 (mos-1162). Design notes written before 197
|
||||
lands, read at `origin/refactor` 01e1fe512. Recheck the line cites against 197's landing before code
|
||||
starts.
|
||||
|
||||
## The defect
|
||||
|
||||
`writeFileSync(file, data, { mode })` and `mkdirSync(dir, { mode })` apply the process umask to the
|
||||
mode. A test that creates a "too open" file this way and expects a refusal passes under `umask 022`
|
||||
(0644 stays 0644) and fails under `umask 077` (0644 becomes 0600, and the source accepts it).
|
||||
|
||||
Row 197 r6 (df0c59608) runs each gate step under `umask 022` (`gate-lane.mjs:46`), so the lane no
|
||||
longer hits this. Row 243 still makes the tests independent of the umask, as the brief asks.
|
||||
|
||||
## Sweep
|
||||
|
||||
Method: every source-side mode check in `packages/*/src` and `scripts/` (grep for `mode & 0o...`), then
|
||||
the tests that reach each check's refusal branch, deciding whether the too-open object gets its mode at
|
||||
creation or from an explicit chmod. Also a scan of every `writeFileSync`/`mkdirSync`/`openSync` in
|
||||
`packages/*/tests` with a group or other bit in its mode (50 hits, 41 without a following chmod), and a
|
||||
grep of the refusal messages for objects created with no mode or by a shell-out. Read at
|
||||
`origin/refactor` with `git show` and `git grep`; nothing run.
|
||||
|
||||
Affected refusal tests, all three verified by reading the test (the fourth file, an accept test, follows the table):
|
||||
|
||||
| Creation | Test | Source check | Expected refusal |
|
||||
|---|---|---|---|
|
||||
| `packages/bus/tests/process.test.mjs:76` `writeFileSync(file, secret, { mode: 0o644 })` | startup token refusal returns safe code... | `bus/src/credentials.mjs:205` (must be 0600) | `credential-file`, exit 2 (:83-84) |
|
||||
| `packages/webui/tests/proof-cli.test.mjs:28` `writeFileSync(file, value, { mode })`, mode 0o644 from the `mode` case at :27 | proof startup refuses partial flags... | `webui/src/serve.mjs:234` (0600 or 0400) | status 2 (:38) |
|
||||
| `packages/webui/tests/sso.test.mjs:360` `writeFileSync(.../secrets/mosaic-stack/a, ..., { mode: 0o644 })` | 11: the CLI refuses --sso... | `webui/src/sso.mjs:45` (no group or other bits) | status 2 and the "no group or other permission bits" line (:361-363); the chmod to 0600 at :364 comes after |
|
||||
|
||||
Fix for each: keep the write, then `chmodSync(<path>, 0o644)` right after it. For `proof-cli.test.mjs`
|
||||
the chmod goes inside the case map after the write, with the case's own mode, so the 0o600 cases are set
|
||||
explicitly too.
|
||||
|
||||
Not affected (each sets the mode with a chmod or a chmodding helper before the code under test reads it):
|
||||
bus credentials.test :52, credential-store.test :55-61; business access.test :225-226, business.test
|
||||
:159/161/174, credentials.test :60, cli.test :151, helpers `writeJson`/`tokenFile`; cli cli.test :176,
|
||||
notifier.test :318/:321, stack-release.test :1220/1239/1294/1313/1552, stack-upgrade.test :426-430,
|
||||
case2-unit.test :340/:510/:635; control-board discord.test :79; discord binding.test :106/118/140/277,
|
||||
notify.test :76, setspark.test :110, git.test :138/:360, offline-check.test :21; mosaic safety.test :80;
|
||||
queue review.test :214/:228; ledger gitea-helper-raw.test :113; webui service.test :797, sso.test :310;
|
||||
bus host-tools.test :182/:266. The 0o644/0o755 adapter and fake-binary writes (launcher, thread-launch,
|
||||
session, sandbox, checkout, commit, etc.) aren't read by a mode check, or are only checked for the exec
|
||||
bit, which 0700 keeps. `host-tools.test.mjs:205`, `:271` and the confDir at `:77` expect acceptance and
|
||||
only need "not group-writable".
|
||||
|
||||
Mode checks with no refusal test at all (not this row; listed for the record): `bus/src/store.mjs:121`,
|
||||
`bus/src/host-run.mjs:71`, `business/src/organization.mjs:28/45/109/141`,
|
||||
`conversation/src/controller.mjs:299`, `mosaic/src/materialize-fixture.mjs:16`,
|
||||
`mosaic/src/refresh-fixture.mjs:64`, `seat/src/session.mjs:72`, `scripts/git-credential-mosaic.mjs:49`,
|
||||
and the mode branch of `scripts/stack-release.sh:1952`.
|
||||
|
||||
Fourth file, folded in by Sage (sage-1918): `packages/cli/tests/stack-instances.test.mjs:84` and `:89`
|
||||
assert 0644 on the config and `instance.json`. `scripts/mosaic-config.mjs:337` creates them with
|
||||
`openSync(file, "wx", 0o644)` and no chmod, so under `umask 077` they come out 0600 and the test fails.
|
||||
This is an accept test of a source mode, not a refusal test.
|
||||
|
||||
| Creation | Test | Source | Ruling |
|
||||
|---|---|---|---|
|
||||
| `scripts/mosaic-config.mjs:337` via `bootstrap()` (`stack-instances.test.mjs:41`, a `spawnSync` of `scripts/bootstrap.sh`) | the `--instance` create test, :79-89 | `createExclusive` passes 0o644 and the umask narrows it | test-only (sage-1918) |
|
||||
|
||||
- No change to `mosaic-config.mjs`. Letting the operator's umask narrow the config is the source's
|
||||
choice; a chmod to 0644 would make the config wider than that umask. No row asks for it.
|
||||
- `bootstrap()` runs its child under an explicit `umask 022`, the pattern from `gate-lane.mjs:46`:
|
||||
`run("/bin/bash", ["-c", 'umask 022; exec "$@"', "bash", BOOTSTRAP, ...args], homeEnv(home))`. Every
|
||||
`bootstrap()` call goes through it (:79, :99, :102, :119, :127, :136, :140, :152, :165, :169), so the
|
||||
0644 assertions still prove the mode the source asks for, whatever the gate's umask.
|
||||
- New assertion: a `bootstrap --instance` run under `umask 077` makes the config and `instance.json`
|
||||
0600. That pins that the source honors the umask, so a later chmod "fix" fails a test. The directories
|
||||
stay 0700 in both runs (they're created with `mode: 0o700`, which 077 can't narrow).
|
||||
- The `config()` helper (:42) has no mode assertion and stays as it is.
|
||||
|
||||
Tests outside `packages/*/tests`: the five `scripts/foundation/*.test.mjs` files have no open-mode
|
||||
write. `cli.test.mjs:144/148` chmods to 0o000 and 0o600, which doesn't depend on the umask. The
|
||||
`agents/*/work` probe tests are review evidence, not gate steps (`discoverSteps` doesn't run them), so
|
||||
they're out of scope.
|
||||
|
||||
## Tests
|
||||
|
||||
- Each of the four files runs whole twice, once under `umask 022` and once under `umask 077`
|
||||
(`bash -c 'umask 077; node --test <file>'`). Both pass. The brief requires this for process.test.mjs;
|
||||
Sage extended it to all four (sage-1918).
|
||||
- Proof that the change matters: before the change, the same four files under `umask 077` fail on the
|
||||
named assertions. I record that run too, on the slot, so the 077 pass after the change is meaningful.
|
||||
- Order (mos-1162): this lands after 197 r6, so r6's proof ran on the unchanged test.
|
||||
|
||||
## Files (expected)
|
||||
|
||||
`packages/bus/tests/process.test.mjs`, `packages/webui/tests/proof-cli.test.mjs`,
|
||||
`packages/webui/tests/sso.test.mjs`, `packages/cli/tests/stack-instances.test.mjs`. Test-only. No
|
||||
source change.
|
||||
|
||||
## Build (sage-2056: start on row 197's landing a36523713)
|
||||
|
||||
Branch `morgana/row243`, one commit on a36523713. Cites rechecked there: all hold except proof-cli's
|
||||
write, which is now `proof-cli.test.mjs:30` (was :28).
|
||||
|
||||
- `bus/tests/process.test.mjs`: imports `chmodSync`; `chmodSync(file, 0o644)` after the write at :76.
|
||||
- `webui/tests/proof-cli.test.mjs:30`: `chmodSync(file, mode)` after the write, with each case's own
|
||||
mode, so the 0o600 cases are set explicitly too.
|
||||
- `webui/tests/sso.test.mjs:360`: `chmodSync(..., 0o644)` after the write, before the `--sso` start
|
||||
that must refuse.
|
||||
- `cli/tests/stack-instances.test.mjs`: `underUmask(mask, home, args)` runs `scripts/bootstrap.sh`
|
||||
through `/bin/bash -c 'umask <mask>; exec "$@"'`; `bootstrap()` is `underUmask("022", ...)`. New
|
||||
subtest in T-I1: under umask 077 the config dir and root are 0700, and the config and `instance.json`
|
||||
are 0600.
|
||||
- Hash pins: no test or script names any of the four files (a grep for the basenames; positive control
|
||||
`stack-release.test` found in two files). `stack-release.test.mjs:2122` mentions
|
||||
`stack-instances.test.mjs` only in a comment.
|
||||
|
||||
Tests: on Sage's slot (sage-2062, plan approved in sage-2059), 18:55Z. My busy check read CLEAR before
|
||||
and after. Each file ran whole with `node --test --test-concurrency=2` from its package, under
|
||||
`bash -c 'umask <mask>; exec ...'`, with TMPDIR from `mktemp -d /mnt/storage/scratch/tmp/g243XXXX`.
|
||||
Script `/mnt/storage/scratch/morgana-r243/slot243.sh`, logs in `logs/` next to it.
|
||||
|
||||
| Tree | umask | process | proof-cli | sso | stack-instances |
|
||||
|---|---|---|---|---|---|
|
||||
| base a36523713 | 077 | 7/1 | 2/1 | 27/1 | 21/2 |
|
||||
| candidate 137bda3af | 022 | 8/0 | 3/0 | 28/0 | 24/0 |
|
||||
| candidate 137bda3af | 077 | 8/0 | 3/0 | 28/0 | 24/0 |
|
||||
|
||||
(pass/fail.) The base fails are the predicted ones: "startup token refusal returns safe code...",
|
||||
"proof startup refuses partial flags...", "11: the CLI refuses --sso...", and in stack-instances the
|
||||
0644 subtest "alpha and beta get a 0700 config dir..." plus its parent T-I1 (so 2). stack-instances has
|
||||
one more test on the candidate (the umask 077 subtest). No unpredicted failure.
|
||||
@@ -1,7 +1,7 @@
|
||||
import test from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { fork } from 'node:child_process';
|
||||
import { mkdtempSync, rmSync, existsSync, writeFileSync, mkdirSync, readFileSync } from 'node:fs';
|
||||
import { mkdtempSync, rmSync, existsSync, writeFileSync, mkdirSync, readFileSync, chmodSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { once } from 'node:events';
|
||||
@@ -74,6 +74,8 @@ test('startup token refusal returns safe code without value or partial listening
|
||||
const secret = 'fixture-token-never-in-db-948723';
|
||||
const file = join(root, 'secret');
|
||||
writeFileSync(file, secret, { mode: 0o644 });
|
||||
// writeFileSync applies the umask; chmod sets the too-open mode under any umask (row 243).
|
||||
chmodSync(file, 0o644);
|
||||
const config = structuredClone(businesses);
|
||||
config.demo.roles.cto.credentials = { gitea: { file, rotateBy: '2099-01-01' } };
|
||||
const { child, logs } = launch(t);
|
||||
|
||||
@@ -37,7 +37,10 @@ function run(cmd, args, env) {
|
||||
return { code: r.status, out: r.stdout, err: r.stderr };
|
||||
}
|
||||
|
||||
const bootstrap = (home, ...args) => run(BOOTSTRAP, args, homeEnv(home));
|
||||
// The child runs under an explicit umask, so the mode assertions hold under any gate umask (row 243,
|
||||
// sage-1918). mosaic-config.mjs creates with 0644 and lets the umask narrow it.
|
||||
const underUmask = (mask, home, args) => run("/bin/bash", ["-c", `umask ${mask}; exec "$@"`, "bash", BOOTSTRAP, ...args], homeEnv(home));
|
||||
const bootstrap = (home, ...args) => underUmask("022", home, args);
|
||||
const config = (home, extra, ...args) => run(process.execPath, [CONFIG, ...args], homeEnv(home, extra));
|
||||
const cfgOf = (home, name) => join(home, ".config", `mosaic-${name}`, "config.json");
|
||||
|
||||
@@ -94,6 +97,16 @@ test("T-I1 bootstrap --instance creates each instance's config, root and marker
|
||||
assert.equal(existsSync(join(home, ".mosaic-dev")), false);
|
||||
});
|
||||
|
||||
await t.test("under umask 077 the config and marker are 0600: the source honors the operator's umask", (t) => {
|
||||
const home = tmp(t, "si-");
|
||||
const r = underUmask("077", home, ["--instance", "alpha"]);
|
||||
assert.equal(r.code, 0, r.err);
|
||||
assert.equal(mode(join(home, ".config", "mosaic-alpha")), 0o700);
|
||||
assert.equal(mode(cfgOf(home, "alpha")), 0o600);
|
||||
assert.equal(mode(join(home, ".mosaic-alpha")), 0o700);
|
||||
assert.equal(mode(join(home, ".mosaic-alpha", "instance.json")), 0o600);
|
||||
});
|
||||
|
||||
await t.test("a second bootstrap of each changes no byte, mode or mtime", (t) => {
|
||||
const home = tmp(t, "si-");
|
||||
for (const name of ["alpha", "beta"]) assert.equal(bootstrap(home, "--instance", name).code, 0);
|
||||
|
||||
@@ -27,7 +27,7 @@ test('proof startup refuses partial flags non-preview and invalid fixture file m
|
||||
const cases = [
|
||||
...sets.map(flags => ({ flags })),
|
||||
...[['mode', VALUE, 0o644], ['short', 'short_fixture', 0o600], ['newline', VALUE.slice(0, 20) + '\n' + VALUE.slice(20), 0o600], ['bad', VALUE + '!', 0o600], ['two-final-lines', VALUE + '\n\n', 0o600]].map(([name, value, mode]) => {
|
||||
const file = join(f.root, name); writeFileSync(file, value, { mode });
|
||||
const file = join(f.root, name); writeFileSync(file, value, { mode }); chmodSync(file, mode); // umask-proof (row 243)
|
||||
return { flags: [...PREVIEW, '--proof-header', 'x-console-proof', '--proof-file', file], value };
|
||||
}),
|
||||
];
|
||||
|
||||
@@ -358,6 +358,7 @@ test('11: the CLI refuses --sso with the old flags and without console.sso, and
|
||||
assert.match(noKey.stderr, /^refused: --sso needs console\.sso in the system config$/m);
|
||||
writeFileSync(env.MOSAIC_CONFIG, JSON.stringify({ ...base, console: { sso: { publicOrigin: ORIGIN, providers: [providerFor(a)] } } }));
|
||||
writeFileSync(join(root, 'cfg', 'secrets', 'mosaic-stack', 'a'), `${a.clientSecret}\n`, { mode: 0o644 });
|
||||
chmodSync(join(root, 'cfg', 'secrets', 'mosaic-stack', 'a'), 0o644); // writeFileSync applies the umask (row 243)
|
||||
const wide = spawnSync(process.execPath, [CLI, 'serve', '--port', '0', '--sso'], { encoding: 'utf8', timeout: 10000, env });
|
||||
assert.equal(wide.status, 2);
|
||||
assert.match(wide.stderr, /^refused: console\.sso provider a client secret slot must have no group or other permission bits$/m);
|
||||
|
||||
Reference in new issue
Block a user