diff --git a/apps/desktop/DESIGN.md b/apps/desktop/DESIGN.md index 11c6e2336..bd3e33708 100644 --- a/apps/desktop/DESIGN.md +++ b/apps/desktop/DESIGN.md @@ -380,7 +380,7 @@ Each is small enough for one PR. 7. **R5b** Activity reader: `hack activity --json [--project] [--limit]`. 8. **R6** Action progress: `hack up|down|restart --json --progress` emitting ndjson `{type:"progress", service, phase}` before the final envelope. 9. **R7a — exists.** Doctor audit schema 1 adds per-check `area`, `scope: machine | project` and `fix: {kind: safe | admin | manual, args[], doc_url}` inside the existing `{ok,data}` envelope. Unknown checks default to Other/manual with no executable args. A category is guidance, not permission to run it. -10. **R7b** One fix without prompts: `hack doctor --fix --only --yes --json`, only for `kind: safe`. +10. **R7b — exists.** `hack doctor --fix --only --yes --json` executes one allowlisted safe fix and rechecks it. Initially only `hack gitignore` is safe; other IDs and mixed modes refuse before writes. Results use `{ok,data:{schema:1,checks,summary,counts,fix}}`; `fix` records `id`, `changed`, `confirmed`. 11. **R7c** Machine-only doctor: `hack doctor --machine --json` (no project), so the app does not need to de-duplicate. 12. **R8** Daemon unit staleness: `service.unit_stale` and reason in `hack daemon status --json`, and `version` there. 13. **R9** (optional) Push updates: `GET /v1/projects?wait=` long-poll. diff --git a/docs/reference/cli.md b/docs/reference/cli.md index af5b832cf..8980a6129 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -1623,6 +1623,8 @@ hack doctor [options] | --- | --- | | `--path, -p ` | Run a project command against a repo path (overrides cwd search) | | `--fix` | Attempt safe auto-remediations (network + CoreDNS + CA) | +| `--only ` | With --fix --yes, run exactly one allowlisted safe repair | +| `--yes` | Consent to the one safe repair selected with --fix --only | | `--migrate-env-config` | Migrate legacy hack.env.json/.env state to the new env config files | | `--accept-tracked-env-changes` | With --migrate-env-config and no prompt, allow deleting or editing git-tracked files (hack.env.json, the Compose file) | | `--domain-migration preview|apply|rollback` | Preview, apply, or roll back only this project's hack.local migration | diff --git a/docs/versions/v5/troubleshooting.md b/docs/versions/v5/troubleshooting.md index a76eac28b..358cdb8fa 100644 --- a/docs/versions/v5/troubleshooting.md +++ b/docs/versions/v5/troubleshooting.md @@ -8,6 +8,8 @@ an original key, not an automatic repair. `hack doctor --json` returns `{ok:true,data:{schema:1,checks,summary,counts}}`. Each check adds an area, machine/project scope and a fix category (`safe`, `admin` or `manual`), argv after `hack`, and a documentation URL. Empty argv means consult the docs. The audit remains read-only; `ok:true` means the audit ran, while error findings still exit 1. Unknown checks offer no runnable repair. +For a targeted repair, `hack doctor --fix --only 'hack gitignore' --yes --json --path ` repairs only Hack's managed ignore block, keeping user lines. It needs a Git checkout and leaves adopted, unsafe or read-only inputs alone. It rechecks the result and exits 1 when unconfirmed. `data.fix` reports `id`, `changed` and `confirmed`; no other check is run or repaired. Other IDs, absent consent and combinations with other repair modes refuse before writes. Broad `--fix --json` still refuses. + The rows marked **next.17 source** describe merged code intended for next.17. They do not change an installed next.16 bundle. See the [release notes](../guides/v5-release-notes.md) for version boundaries. diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 76eb83c91..f9c17b8f5 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -152,6 +152,10 @@ import { NATIVE_SOURCE_CHECK, type NativeSourceReport, } from "../lib/doctor-native-source.ts"; +import { + DOCTOR_SAFE_FIX_ID, + runDoctorSafeFix, +} from "../lib/doctor-safe-fix.ts"; import { inspectSecretIntegrity } from "../lib/doctor-secret-integrity.ts"; import { findCrossCheckoutInstances, @@ -430,6 +434,19 @@ const optBrowserResult = defineOption({ const doctorOptions = [ optPath, optFix, + defineOption({ + name: "only", + type: "string", + long: "--only", + valueHint: "", + description: "With --fix --yes, run exactly one allowlisted safe repair", + } as const), + defineOption({ + name: "yes", + type: "boolean", + long: "--yes", + description: "Consent to the one safe repair selected with --fix --only", + } as const), optMigrateEnvConfig, optAcceptTrackedEnvChanges, optDomainMigration, @@ -741,6 +758,15 @@ async function runNativePreflightMode( const handleDoctor: CommandHandlerFor = async ({ args, }): Promise => { + if (selectedDoctorFixRequested(args.options)) { + return await runSelectedDoctorFix(args); + } + return await runDoctorAudit(args); +}; + +async function runDoctorAudit( + args: Parameters>[0]["args"] +): Promise { if (args.options.nativePreflight) { return await runNativePreflightMode(args); } @@ -1140,7 +1166,58 @@ const handleDoctor: CommandHandlerFor = async ({ } return 0; -}; +} + +function selectedDoctorFixRequested(opts: { + readonly only?: string; + readonly yes?: boolean; +}): boolean { + return opts.only !== undefined || opts.yes === true; +} + +/** An allowlist before discovery or side effects: new Doctor flags cannot widen a selected repair. */ +async function runSelectedDoctorFix( + args: Parameters>[0]["args"] +): Promise { + const allowed = new Set(["path", "fix", "only", "yes", "json"]); + if ( + Object.entries(args.options).some( + ([key, value]) => + !allowed.has(key) && value !== undefined && value !== false + ) || + !args.options.fix || + !args.options.yes || + args.options.only !== DOCTOR_SAFE_FIX_ID + ) { + throw new CliUsageError( + "Use --fix --only 'hack gitignore' --yes for the supported safe repair; other check IDs and repair modes are refused." + ); + } + const started = performance.now(); + const fixed = await runDoctorSafeFix({ + startDir: resolveDoctorStartDir(args.options.path), + }); + const results = [ + { ...fixed.check, durationMs: Math.round(performance.now() - started) }, + ]; + if (args.options.json) { + emitCliResult({ + result: okResult({ + data: { + ...buildDoctorJsonData({ results }), + fix: { + id: DOCTOR_SAFE_FIX_ID, + changed: fixed.changed, + confirmed: fixed.confirmed, + }, + }, + }), + }); + } else { + await renderDoctorSummary(results); + } + return fixed.confirmed ? 0 : 1; +} /** * Compose doctor's browser check concerns the reported origin, else the diff --git a/src/lib/doctor-metadata.ts b/src/lib/doctor-metadata.ts index 409e7440c..75bd38a20 100644 --- a/src/lib/doctor-metadata.ts +++ b/src/lib/doctor-metadata.ts @@ -119,13 +119,32 @@ export function doctorMetadata(opts: { "native root trust", "native ca repair", ].includes(opts.id); + const fix: DoctorMetadata["fix"] = { + kind: adminTrust ? "admin" : "manual", + args: opts.id === "local ca trust" ? ["global", "trust"] : [], + doc_url: DOC_URL, + }; + if (opts.id === "hack gitignore") { + return { + area, + scope, + fix: { + kind: "safe", + args: [ + "doctor", + "--fix", + "--only", + "hack gitignore", + "--yes", + "--json", + ], + doc_url: DOC_URL, + }, + }; + } return { area, scope, - fix: { - kind: adminTrust ? "admin" : "manual", - args: opts.id === "local ca trust" ? ["global", "trust"] : [], - doc_url: DOC_URL, - }, + fix, }; } diff --git a/src/lib/doctor-safe-fix.ts b/src/lib/doctor-safe-fix.ts new file mode 100644 index 000000000..a400f2392 --- /dev/null +++ b/src/lib/doctor-safe-fix.ts @@ -0,0 +1,71 @@ +import { HackCliError } from "./cli-result.ts"; +import { preflightDoctorProject } from "./doctor-adopted-project.ts"; +import { inspectHackDirGitignoreForDoctor } from "./doctor-generated-files.ts"; +import { findProjectContext } from "./project.ts"; +import { ensureHackDirGitignore } from "./project-env-config.ts"; + +export const DOCTOR_SAFE_FIX_ID = "hack gitignore"; + +/** Select an ordinary legacy project again at publication; adoption never authorizes this write. */ +async function selectedProject(startDir: string) { + if (await preflightDoctorProject({ startDir })) { + throw new HackCliError({ + code: "E_CONFIG_INVALID", + message: "Selected repair does not change adopted project inputs.", + }); + } + const project = await findProjectContext(startDir); + if (!project) { + throw new HackCliError({ + code: "E_PROJECT_NOT_FOUND", + message: "Selected repair requires a project in a Git checkout.", + }); + } + return project; +} + +/** Exactly one additive repair, followed by readback. Never calls the broad guided remediation. */ +export async function runDoctorSafeFix(opts: { readonly startDir: string }) { + const project = await selectedProject(opts.startDir); + const inspect = () => + inspectHackDirGitignoreForDoctor({ projectDir: project.projectDir }); + const before = await inspect(); + if (before === null) { + throw new HackCliError({ + code: "E_CONFIG_INVALID", + message: "Selected repair requires a Git checkout.", + }); + } + let changed = false; + if (before === "missing" || before === "outdated") { + const result = await ensureHackDirGitignore({ + projectDir: project.projectDir, + beforePublish: async () => { + const current = await selectedProject(opts.startDir); + if ( + current.projectRoot !== project.projectRoot || + current.projectDir !== project.projectDir + ) { + throw new HackCliError({ + code: "E_CONFIG_INVALID", + message: "Selected repair project changed; nothing published.", + }); + } + }, + }); + changed = result.changed; + } + const after = await inspect(); + const confirmed = after === "current"; + return { + changed, + confirmed, + check: { + name: DOCTOR_SAFE_FIX_ID, + status: confirmed ? ("ok" as const) : ("error" as const), + message: confirmed + ? "Managed ignore block verified; existing user lines preserved." + : "Selected repair could not verify the managed ignore block; unsafe, unreadable and read-only files are left unchanged.", + }, + }; +} diff --git a/tests/doctor-safe-fix.test.ts b/tests/doctor-safe-fix.test.ts new file mode 100644 index 000000000..c359c22a5 --- /dev/null +++ b/tests/doctor-safe-fix.test.ts @@ -0,0 +1,176 @@ +import { expect, test } from "bun:test"; +import { + chmod, + mkdir, + mkdtemp, + readdir, + readFile, + rm, + symlink, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { runFixtureGit } from "./helpers/fixture-git.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; + +const cli = resolve(import.meta.dir, "../index.ts"); +const selected = ["--fix", "--only", "hack gitignore", "--yes", "--json"]; + +async function fixture() { + const root = await mkdtemp(join(tmpdir(), "doctor-safe-fix-")); + const project = join(root, "project"); + await mkdir(join(project, ".hack"), { recursive: true }); + await runFixtureGit({ cwd: project, args: ["init", "--quiet"] }); + await Bun.write(join(project, ".hack/docker-compose.yml"), "services: {}\n"); + const tools = join(root, "tools"); + await mkdir(tools); + const called = join(root, "unexpected-tool"); + for (const name of ["docker", "hackd", "sudo", "security", "systemctl"]) { + await Bun.write( + join(tools, name), + `#!/bin/sh\nprintf unexpected >> '${called}'\nexit 90\n` + ); + await chmod(join(tools, name), 0o700); + } + const invoke = async (flags: string[]) => { + const child = Bun.spawn( + [ + process.execPath, + "--no-env-file", + cli, + "doctor", + "--path", + project, + ...flags, + ], + { + cwd: root, + env: privateCliEnv({ + root, + env: { + HACK_HOME: join(root, "state"), + HACK_GLOBAL_CONFIG_PATH: join(root, "state/hack.config.json"), + PATH: `${tools}:/usr/bin:/bin`, + HACK_NO_INTERACTIVE: "1", + HACK_LOGGER: "console", + }, + }), + stdout: "pipe", + stderr: "pipe", + } + ); + const [code, stdout, stderr] = await Promise.all([ + child.exited, + new Response(child.stdout).text(), + new Response(child.stderr).text(), + ]); + expect(await Bun.file(called).exists()).toBe(false); + return { code, stdout, stderr, json: JSON.parse(stdout) }; + }; + return { root, project, invoke, file: join(project, ".hack/.gitignore") }; +} + +test("one safe fix preserves user lines and other findings, rechecks, and is idempotent", async () => { + const f = await fixture(); + try { + await Bun.write(f.file, "# user-owned rule\ncustom.tmp\n"); + const unrelated = join(f.project, ".hack/.env"); + await Bun.write(unrelated, "SYNTHETIC_SENTINEL=not-a-real-secret\n"); + const result = await f.invoke(selected); + expect(result.code).toBe(0); + expect(result.stderr).toBe(""); + expect(Object.keys(result.json)).toEqual(["ok", "data"]); + expect(result.json).toMatchObject({ + ok: true, + data: { + schema: 1, + fix: { id: "hack gitignore", changed: true, confirmed: true }, + counts: { ok: 1, warn: 0, error: 0 }, + checks: [{ id: "hack gitignore", status: "ok", fix: { kind: "safe" } }], + }, + }); + expect(result.json.data.checks).toHaveLength(1); + expect(await Bun.file(f.file).text()).toContain( + "# user-owned rule\ncustom.tmp\n" + ); + expect(await Bun.file(unrelated).text()).toBe( + "SYNTHETIC_SENTINEL=not-a-real-secret\n" + ); + expect(result.stdout).not.toContain("SYNTHETIC_SENTINEL"); + const before = await readFile(f.file); + const again = await f.invoke(selected); + expect(again.code).toBe(0); + expect(again.json.data.fix).toMatchObject({ + changed: false, + confirmed: true, + }); + expect(await readFile(f.file)).toEqual(before); + } finally { + await rm(f.root, { recursive: true, force: true }); + } +}); + +test("missing consent, broad fix, admin/manual/unknown IDs and mixed modes refuse before mutation", async () => { + const f = await fixture(); + try { + for (const flags of [ + ["--fix", "--only", "hack gitignore", "--json"], + ["--only", "hack gitignore", "--yes", "--json"], + ["--fix", "--yes", "--json"], + ["--fix", "--json"], + ...["local ca trust", "daemon", "unknown", "__proto__"].map((id) => [ + "--fix", + "--only", + id, + "--yes", + "--json", + ]), + [...selected, "--migrate-env-config"], + [...selected, "--native-preflight"], + ]) { + const result = await f.invoke(flags); + expect(result.code).not.toBe(0); + expect(result.json).toMatchObject({ + ok: false, + error: { code: "E_USAGE" }, + }); + expect(await Bun.file(f.file).exists()).toBe(false); + expect(await readdir(join(f.project, ".hack"))).toEqual([ + "docker-compose.yml", + ]); + } + } finally { + await rm(f.root, { recursive: true, force: true }); + } +}); + +for (const mode of ["read-only", "symlink"] as const) { + test(`a ${mode} ignore file is retained and cannot be reported fixed`, async () => { + const f = await fixture(); + try { + const target = join(f.root, "user-content"); + await Bun.write(target, "user content\n"); + if (mode === "symlink") { + await symlink(target, f.file); + } else { + await Bun.write(f.file, "user rule\n"); + await chmod(f.file, 0o400); + } + const result = await f.invoke(selected); + expect(result.code).toBe(1); + expect(result.json).toMatchObject({ + ok: true, + data: { + fix: { changed: false, confirmed: false }, + checks: [{ status: "error" }], + }, + }); + expect(await Bun.file(target).text()).toBe("user content\n"); + expect(await Bun.file(f.file).text()).toBe( + mode === "symlink" ? "user content\n" : "user rule\n" + ); + } finally { + await rm(f.root, { recursive: true, force: true }); + } + }); +}