fix(#804): reject unknown installer arguments
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful

This commit is contained in:
Hermes Agent
2026-07-17 16:53:02 -05:00
parent ed48ea0399
commit a1b2e8d66b
6 changed files with 108 additions and 29 deletions

View File

@@ -349,6 +349,8 @@ bash tools/install.sh --yes # Non-interactive, accept all defaults
bash tools/install.sh --no-auto-launch # Skip auto-launch of wizard
```
The installer rejects unrecognized flags or positional arguments before making changes and prints the supported-option usage.
## Contributing
```bash

View File

@@ -183,6 +183,8 @@ non-interactive use:
--no-auto-launch # Skip auto-launch of wizard after install
```
Unrecognized flags or positional arguments fail before installation starts and print the supported-option usage.
Or if installed globally:
```bash

View File

@@ -43,12 +43,44 @@ Implement Part 1 of Gitea issue #804 only: `tools/install.sh` must reject every
- 2026-07-17: Confirmed the parser has no legitimate positional arguments and currently drops all unmatched input via `*) shift ;;`.
- 2026-07-17: Installed locked workspace dependencies with a worktree-local pnpm store; no lockfile changes.
- 2026-07-17: Added the process-level unknown-argument regression with an isolated `$HOME` and npm shim.
- 2026-07-17: Replaced the silent catch-all with STDERR error + usage output and exit 2 before preflight or installation.
- 2026-07-17: Initial Codex code review found an unknown option could still be consumed as the `--ref` value. Added a second RED reproducer, then rejected option-shaped/missing `--ref` values without changing valid `--ref <branch>` behavior. The review's launcher-state note is handled by excluding both `.mosaic/orchestrator/` files from commits.
- 2026-07-17: Updated README, user guide, and packaged framework README with the fail-closed argument contract. No API, auth, admin, sitemap/navigation, or publishing surface changed.
## Verification
- RED: `pnpm --filter @mosaicstack/mosaic exec vitest run src/commands/install-arguments.spec.ts` — expected failure: installer exited `0` instead of nonzero at the exit-status assertion; confirms the test reproduces the silent-drop defect before production changes.
- Remediation RED: the added `--cli --ref --bogus` case exited `0`, proving `--ref` could swallow an unknown option before the guard was added.
- GREEN: `pnpm --filter @mosaicstack/mosaic exec vitest run src/commands/install-arguments.spec.ts src/commands/install-heading.spec.ts` — 2 files, 3 tests passed.
- Situational process check: unknown positional input exited 2, named the input on STDERR, printed usage, and did not call the npm shim.
- `bash -n tools/install.sh` — passed.
- Bare `--ref` process check — exited 2 with `Missing value for --ref` and usage.
- `pnpm --filter @mosaicstack/mosaic test` — 69 files, 1,287 tests passed; framework shell checks passed. The first attempt lacked generated `dist/cli.js`; `pnpm --filter @mosaicstack/mosaic build` restored the required test precondition and the full rerun passed.
- `pnpm lint` — 23/23 tasks passed.
- `pnpm typecheck` — 42/42 tasks passed.
- `pnpm format:check` — passed.
- Codex code re-review against `origin/main``approve`, 0 blockers/should-fix/suggestions.
- Codex security re-review against `origin/main` — risk `none`, 0 findings.
## Acceptance evidence
| Criterion | Evidence |
| --- | --- |
| Unknown input is named on STDERR | Process-level Vitest assertions for `--bogus`, including after `--ref` |
| Short usage hint is printed on STDERR | Vitest usage regex + manual process output |
| Exit is nonzero | Vitest status assertions and manual exit 2 |
| Installation does not proceed | Isolated npm shim marker remains absent |
| Recognized behavior is preserved | Parser cases are unchanged except validation of malformed `--ref`; full Mosaic package suite passed |
| Part 2 is excluded | No `--next`, `MOSAIC_NEXT`, dist-tag, or prerelease routing changes |
## Documentation checklist
- Current canonical `docs/PRD.md` remains unchanged; issue #804 and the coordinator brief supply this bounded defect's acceptance contract.
- Updated installer behavior in root README, user guide, and packaged framework README in the same logical change set.
- API/OpenAPI, auth/permissions, admin operations, developer architecture, sitemap/navigation, and external publishing are not affected.
- Scratchpad remains under `docs/scratchpads/`; no root-hygiene changes.
## Risks and blockers
- Part 2 remains owner-gated under #805 and is intentionally excluded.
- Test harness must isolate `$HOME` and shim npm so the pre-fix installer cannot mutate operator state or contact the registry.
- No implementation blocker remains. Independent coordinator RoR, CI, merge, and issue closure remain pending after worker handoff.

View File

@@ -177,6 +177,8 @@ bash tools/install.sh --cli # npm CLI only (skip framework)
bash tools/install.sh --ref v1.0 # Install from a specific git ref
```
The installer rejects unrecognized flags or positional arguments before making changes and prints the supported-option usage.
## Universal Skills
The installer syncs skills from `mosaic/agent-skills` into `~/.config/mosaic/skills/`, then links each skill into runtime directories.

View File

@@ -8,8 +8,11 @@ import { describe, expect, it } from 'vitest';
const INSTALLER_PATH = fileURLToPath(new URL('../../../../tools/install.sh', import.meta.url));
describe('installer arguments', () => {
it('rejects an unknown argument before installation starts', async () => {
async function runInstaller(args: string[]): Promise<{
status: number | null;
stderr: string;
npmCalled: boolean;
}> {
const home = await mkdtemp(join(tmpdir(), 'mosaic-installer-args-'));
const bin = join(home, 'bin');
const npmMarker = join(home, 'npm-called');
@@ -20,7 +23,7 @@ describe('installer arguments', () => {
await writeFile(npmShim, `#!/bin/sh\n: > "${npmMarker}"\n`);
await chmod(npmShim, 0o755);
const result = spawnSync('bash', [INSTALLER_PATH, '--cli', '--bogus'], {
const result = spawnSync('bash', [INSTALLER_PATH, ...args], {
encoding: 'utf8',
env: {
...process.env,
@@ -30,12 +33,29 @@ describe('installer arguments', () => {
},
});
expect(result.status).not.toBe(0);
expect(result.stderr).toContain('Unknown argument: --bogus');
expect(result.stderr).toMatch(/Usage: .*install\.sh/);
expect(existsSync(npmMarker)).toBe(false);
return {
status: result.status,
stderr: result.stderr,
npmCalled: existsSync(npmMarker),
};
} finally {
await rm(home, { recursive: true, force: true });
}
}
function expectUnknownArgument(result: Awaited<ReturnType<typeof runInstaller>>): void {
expect(result.status).not.toBe(0);
expect(result.stderr).toContain('Unknown argument: --bogus');
expect(result.stderr).toMatch(/Usage: .*install\.sh/);
expect(result.npmCalled).toBe(false);
}
describe('installer arguments', () => {
it('rejects an unknown argument before installation starts', async () => {
expectUnknownArgument(await runInstaller(['--cli', '--bogus']));
});
it('does not let --ref consume an unknown option', async () => {
expectUnknownArgument(await runInstaller(['--cli', '--ref', '--bogus']));
});
});

View File

@@ -61,17 +61,38 @@ if [[ "${MOSAIC_DEV:-0}" == "1" ]]; then
FLAG_DEV=true
fi
installer_usage() {
printf 'Usage: install.sh [--check] [--framework] [--cli] [--ref <branch>] [--dev] [--yes|-y] [--no-auto-launch] [--uninstall]\n' >&2
}
while [[ $# -gt 0 ]]; do
case "$1" in
--check) FLAG_CHECK=true; shift ;;
--framework) FLAG_CLI=false; shift ;;
--cli) FLAG_FRAMEWORK=false; shift ;;
--ref) GIT_REF="${2:-main}"; shift 2 ;;
--ref)
if [[ $# -lt 2 ]] || [[ -z "$2" ]]; then
printf 'Error: Missing value for --ref\n' >&2
installer_usage
exit 2
fi
if [[ "$2" == -* ]]; then
printf 'Error: Unknown argument: %s\n' "$2" >&2
installer_usage
exit 2
fi
GIT_REF="$2"
shift 2
;;
--dev) FLAG_DEV=true; shift ;;
--yes|-y) FLAG_YES=true; shift ;;
--no-auto-launch) FLAG_NO_AUTO_LAUNCH=true; shift ;;
--uninstall) FLAG_UNINSTALL=true; shift ;;
*) shift ;;
*)
printf 'Error: Unknown argument: %s\n' "$1" >&2
installer_usage
exit 2
;;
esac
done