diff --git a/CHANGELOG.md b/CHANGELOG.md index 876a25e2..a9e93431 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,46 @@ # Changelog +## 1.0.0 — 2026-08-12 + +The first stable release. Everything below this heading shipped across the +`1.0.0-beta.*` line, which stays documented in its own sections underneath. + +Two things are worth stating plainly for anyone arriving at 1.0.0 from a beta. + +**Enforcement runs through a background daemon, and a configured machine fails +closed.** `failproofai config` installs `failproofaid` as a system service; from +that point an unreachable daemon DENIES rather than silently falling back to +in-process evaluation, because a guarantee you can remove by stopping a service +is not one. In-process evaluation still exists and is reachable only on a +machine that was never set up. Linux and macOS only — setup refuses on other +platforms rather than completing with a weaker guarantee. + +**A policy that cannot be honoured on your CLI is not silently pretended.** +`enforcement-capability.ts` records, per (CLI, event) pair and with its evidence, +whether a deny actually changes the agent's behaviour. Absent means UNVERIFIED, +never "blocked". + +### Features + +- Cut setup's prose by two thirds. The daemon step spent three lines explaining the warm-worker architecture to somebody about to type a password; the cloud step spent five on what connecting sends. Eleven lines became three — say what is happening and what it costs, drop the mechanism. The cloud screen keeps the one clause that is not explanation ("Sessions include prompts, file contents and command output"), because it is a consent screen and the only place that disclosure is ever made: `describeOutcome` prints "hook activity" afterwards and never mentions transcripts, so compressing it to "telemetry" would be brevity that is really vagueness. Its option hint now says what the cloud GIVES — central monitoring and policy deployment, which is what the key's two scopes buy — rather than "see what your agents did", which the local dashboard already shows and which made connecting look redundant. The same screen names the product rather than the artefact — "New to FailproofAI? Create a key at befailproof.ai/get-started" — and points at get-started rather than the dashboard host, since somebody reading that line has no key and usually no org either, and "No key?" reads as an error state to a person who has simply not signed up yet. (#683) +- Give `failproofai config` a Recommended path, so the common install is two questions instead of five. Setup opened by asking scope, policy bundles, harnesses and cloud of somebody who has just installed the tool and does not yet know what any of those mean — every one of which has a defensible default, so asking all four up front made the person least able to answer do the most work. Recommended is not a shortcut past those decisions, it is a decision taken on their behalf: global scope (a project install guards the one directory the command was run from and silently leaves every other repo unguarded), the CLIs actually detected on the machine, and a named 15-policy set. Customize is the previous wizard unchanged — nothing is removed and nothing is hidden, it stops being the only way through. The cloud question is still asked on both paths. (#683) +- Name what "Recommended" means, in one list with the reasoning attached. `RECOMMENDED_POLICIES` in `policy-presets.ts` is written out rather than derived from `defaultEnabled`, because those answer different questions — `defaultEnabled` seeds a checklist of 40, this answers "what should guard a machine whose owner did not want to choose" — and deriving one from the other would silently reshape the recommended set every time somebody flipped a flag on an unrelated policy. It is the 12 that were already default-on plus **three that were off and should not have been**: `block-rm-rf`, `block-force-push` and `block-secrets-write`. A recommended setup that omits catastrophic deletion and force-push is not recommendable, and both are precisely scoped — `block-rm-rf` only fires at depth ≤2 under `/` or a home directory, exempts `/tmp`, and treats an unresolved `$VAR` target as catastrophic, so `rm -rf node_modules` is untouched; `block-force-push` blocks `--force` and `-f` while explicitly allowing `--force-with-lease` and `--force-if-includes`. Deliberately excluded, each for a stated reason: the `require-*-before-stop` workflow gates (they refuse to let the agent finish until CI is green, and per `enforcement-capability.ts` do not fire at all on hermes or goose), the infra blockers (they break the day job of anyone who runs kubectl), `block-read-outside-cwd` (agents legitimately read outside the repo) and the ten `warn-*` policies (a warning nobody reads is worse than one that was never shown). (#683) +- Give the review screen a taste of the policy set rather than only a count. `Policies : 15 enabled` is a number the user cannot check and, on the recommended path, did not choose. Two names and a count of the rest now sit under it — `block-curl-pipe-sh, block-env-files +13` — which is the same shape `describeSelection` already uses for bundles, and enough to say what KIND of thing these are without turning a four-line review into a thirteen-line one; a screen nobody reads to the bottom conveys less than a short one. The whole review body is rendered dim by the prompt, so it reads as a subtitle to the count rather than competing with it, and it scales unchanged to "Everything" (`block-aws-cli, block-az-cli +38`). Degrades by dropping a name rather than overflowing: `writeLines` truncates with a hard cut and no ellipsis, so an over-long line ends mid-slug and reads as a policy name that does not exist. (#683) +- Union rather than replace when Recommended writes. `installHooks` runs with `replace: true`, so writing the bare 15 would switch OFF anything the user had enabled by hand — turning "give me the sensible defaults" into a reduction in protection, the one direction this must never move. On a fresh machine the union is exactly the 15. The `customPoliciesEnabled` flag is left alone on this path for the same reason: the customize expression evaluates to `false` when no bundle is ticked, and no bundle is ever ticked here, so writing it would disable every `.failproofai/policies` file on disk as a side effect of choosing the default setup. (#683) + +### Fixes + +- Give a Hermes session one agent id for its whole life. A single session was arriving under two — confirmed on a customer org, `20260812_133702_31ca19f0` under both `hermes-kratos` and `hermes-telegram`, and a cron session under both `hermes-cron` and the bare fallback, from ONE collector with the other producer's rows excluded. The id was derived from the session's own `cwd` and `source` columns and re-read on every poll, and Hermes rewrites those throughout a run (`hermes_state.py` carries ~20 `UPDATE sessions SET …`), so a session split the moment one changed between two polls. The file documented that as a safety property — "session columns are read fresh on every poll" — while the `pending` map directly below states the opposite rule correctly for tool names; reading fresh is right for a name and wrong for an identity. Identity now comes from which DATABASE the session is in: the root keeps the bare `hermes` every deployment already ships under, and `profiles//state.db` becomes `hermes-`. A path cannot change mid-poll, so this is stable by construction, it keeps the poll function pure (the format contract requires that or re-read rows dedup into duplicates), and it matches the standalone collector's `agent_id_for` so a machine migrating off it is not renamed. Nothing is lost: `hermes_source` and `hermes_cwd` were already on every event, so transport and project stay answerable as filters over one agent's sessions. The regression test polls twice with `cwd` rewritten in between — every existing test polled once, which is why a shipping bug sat behind a green suite. (#683) +- Close a hole in the bundle vocabulary that Recommended had to route around. Four `defaultEnabled` policies — `block-self-pause`, `block-sudo`, `block-curl-pipe-sh` and `block-failproofai-commands` — are in the `Dangerous Commands` category, which no preset covers, and only `block-secrets-write` is rescued from it by the secrets preset's `extra`. So 8 of the 12 default-on policies are reachable by ticking bundles and 4 are not, and since the wizard writes with `replace: true`, a first run that picks bundles produces a machine WITHOUT the two policies that stop the agent disabling failproofai itself. `RECOMMENDED_POLICIES` names all 15 explicitly rather than composing bundles, and `defaultsMissingFromRecommended()` plus a test assert that every default-on policy stays in it — so the day a new one is added, the recommended set cannot silently fall behind the checklist. The bundles themselves still have the gap; closing it needs a fifth bundle or a decision that the self-protection policies are not optional. (#683) +- Make `mode: "oss"` actually stop the daemon talking to the cloud. `config --disconnect` writes that flag and its own comment states the rule: "every cloud code path keys off this flag rather than off 'is a token lying around' precisely so that a disconnected machine is provably silent instead of silent-by-happenstance." That was true of the TypeScript CLI and false of the **daemon**, which is the process that holds the socket and had never read the flag — so a machine put back on OSS whose credential file outlived the decision to leave went on polling and shipping while `--status` reported it disconnected: a restored home, a copied config, a reinstall, a partial cleanup, or simply the layout-1 `cloud.json` fallback. The veto is checked inside `from_file` rather than at the call site, because that function has three exits and a veto guarding only some of them is not a veto. It reads `mode.kind` — `fp-config.ts` persists `mode: { kind }`, an object, and reading it as a string is why an earlier cut of this never fired at all; the tests missed it because their fixtures carried the same wrong assumption as the code, which is worse than no test since they also reported the case as covered. ONLY an explicit `"oss"` vetoes: `mode` postdates the enrolments already in the field, so reading absent or malformed as "oss" would silently disconnect every machine enrolled by an older CLI. `FAILPROOFAI_CLOUD_URL` still wins, since the env path exists so CI and containers work with no files at all. (#683) +- Stop reporting a connection the machine no longer has. Everything `config --status` printed about the cloud connection was read from the credential file, which records what was true at `--connect` time and is never revisited — so a key that was later revoked, expired (`api_key_expiry` exists server-side now), or whose org was disabled left that file byte-for-byte correct while nothing arrived. That is the failure recorded in `crates/failproofaid/src/main.rs`: "a key revoked at 13:05:37 and replaced 37 seconds later was still producing 401s twenty minutes on, with 26 parked batches and a CLI saying `connected`. The only symptom was data that never arrived." The detection was never missing — the uploader writes its verdict into the parked batch's **filename** (`.a.c.jsonl`, a rename rather than a sidecar so the record cannot desynchronise from the batch it describes), and `is_auto_retryable()` already excludes a definitively-refused batch from the retry pass because it "will fail identically until the cause is fixed". Nothing had ever read that directory. `--status` now reports from it, and the line replaces the cheerful one rather than being appended after it, because the collector's record of what the server actually said is the only thing on that screen describing **now**. Deliberately silent on batches parked without a client status: those exhausted server-error retries, the retry pass picks them up again, and reporting them would cry wolf over a blip the daemon is already handling. (#683) +- Say it once a session, without anyone having to ask. `--status` only speaks when someone runs it, and the whole failure mode is that nobody knows there is anything to ask about — so the same verdict is now emitted at `SessionStart`, the one point failproofai is already invoked on every CLI, exactly once per session, with a person watching. Costs one directory read per session; needs no flag, no new daemon channel and no user action. It cannot affect the outcome of the hook: the read swallows its own errors, the verdict goes to stderr (`SessionStart` is `observe` on every integration, so a stderr write there cannot block a session), and the exit code is untouched. 401 and 403 are grouped as "credential" — a rejected key and a key without `events:add` have different causes, the same user-visible outcome, and the same fix — and only that group is told to re-run `--connect`. (#683) +- Make a `PostToolUse` deny actually enforce on codex and copilot. Both read a **top-level** `{decision:"block", reason}` at that event and neither reads the `hookSpecificOutput.additionalContext` shape we emitted, so every PostToolUse deny on those two CLIs was evaluated, logged, counted as enforcement in the dashboard — and dropped on the floor. That is the whole `sanitize-*` family plus any custom policy matching `PostToolUse`. Verified rather than inferred, because the rows asserting it were stale in both directions: on codex an A/B live probe at **0.147.0** (identical prompt and hook, only the response shape differing) shows `{decision:"block"}` printing `hook: PostToolUse Blocked` and routing the reason through `codex_core::tools::router` so that it **replaces the tool result the model reads** — the probe's real stdout never reached the model — while the shape we shipped printed `hook: PostToolUse Completed` and the model read that stdout verbatim; on copilot both `postToolUse` call sites in the shipped **1.0.78** bundle gate on `vK = t => t?.decision === "block" && typeof t.reason === "string"`, which fails closed on a missing or non-string reason, so the reason is always sent as a string. This is result-replacement, not prevention — the tool has already run at PostToolUse and its side effect stands — which is exactly the semantic an output-scrubbing policy needs to keep a secret out of the model's context, and is why copilot's "Can block? No" in vendor docs is true of the side effect but not of the result the model reads. Other CLIs are untouched and keep `additionalContext`; a test pins that boundary, since widening the new shape to claude would silently break the one consumer that does read the nested form. (#683) + +### Docs + +- Record that several `codex` rows in `enforcement-capability.ts` are no longer backed by anything. Re-probing codex for the row above surfaced that its hook sources were restructured after `fe01054a`: `output_parser.rs`, `hook_runtime.rs` and `tools/registry.rs` — cited by name and line number in the `PreToolUse`, `Stop`, `SubagentStop` and `SessionStart` rows — do not exist at those paths in 0.147.0, whose hooks live under `hooks/src/{engine,events}/`. The rows are not known-wrong; they are unverified against any shipping codex, which under this file's own "ABSENT MEANS UNKNOWN" rule is a materially different claim. Marked as due for a re-probe, and the probed-version line now states that a version is part of the claim rather than a footnote. (#683) + ## 1.0.0-beta.21 — 2026-08-10 ### Fixes diff --git a/Cargo.lock b/Cargo.lock index 84a2a2ef..b64b8fed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -202,7 +202,7 @@ dependencies = [ [[package]] name = "failproofaid" -version = "1.0.0-beta.22" +version = "1.0.0" dependencies = [ "fpai-collect", "fpai-ipc", @@ -265,7 +265,7 @@ dependencies = [ [[package]] name = "fpai-collect" -version = "1.0.0-beta.22" +version = "1.0.0" dependencies = [ "notify", "reqwest", @@ -280,7 +280,7 @@ dependencies = [ [[package]] name = "fpai-ipc" -version = "1.0.0-beta.22" +version = "1.0.0" dependencies = [ "libc", "proptest", diff --git a/Cargo.toml b/Cargo.toml index f448d58a..d9d37e07 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "3" members = ["crates/*"] [workspace.package] -version = "1.0.0-beta.22" +version = "1.0.0" edition = "2024" license-file = "LICENSE" repository = "https://github.com/FailproofAI/failproofai" diff --git a/README.md b/README.md index 82a0e6a1..a565ee9a 100644 --- a/README.md +++ b/README.md @@ -215,5 +215,4 @@ See [CONTRIBUTING.md](./CONTRIBUTING.md). New policies, edge cases, and translat --- -Built by [Nivedit Jain](https://github.com/NiveditJain) and [Nikita Agarwal](https://github.com/nk-ag). -[befailproof.ai](https://befailproof.ai) +Built with ❤️ by [befailproof.ai](https://befailproof.ai) in SF and Bengaluru. diff --git a/__tests__/audit/scheduled-audit.test.ts b/__tests__/audit/scheduled-audit.test.ts index 840adfd7..fcdb60e6 100644 --- a/__tests__/audit/scheduled-audit.test.ts +++ b/__tests__/audit/scheduled-audit.test.ts @@ -184,7 +184,7 @@ describe("the scan stays on this machine", () => { it("uploads nothing, however much it finds", async () => { // The scheduled scan reads the CONTENTS of every session transcript on // disk — prompts, file contents, pasted credentials, command output. It - // once POSTed a counters-only projection of that to Failproof Cloud; that + // once POSTed a counters-only projection of that to FailproofAI Cloud; that // path is gone, and this pins that it does not come back by accident. // // `fetch` is the seam because it is the only way anything here reaches the diff --git a/__tests__/e2e/helpers/hook-runner.ts b/__tests__/e2e/helpers/hook-runner.ts index caa3919c..7252ebf4 100644 --- a/__tests__/e2e/helpers/hook-runner.ts +++ b/__tests__/e2e/helpers/hook-runner.ts @@ -107,6 +107,26 @@ export function assertPostToolUseDeny(result: HookRunResult): void { expect(output?.additionalContext).toMatch(/Blocked/i); } +/** + * PostToolUse deny on codex and copilot: a TOP-LEVEL `{decision, reason}`. + * + * Deliberately a second assertion rather than widening `assertPostToolUseDeny` + * to accept either shape. The two shapes are the entire substance of the + * change — codex and copilot read the top-level object and ignore the nested + * one, every other CLI does the reverse — so a helper that passed on whichever + * arrived would go green if a CLI were wired to the wrong one, in either + * direction, which is precisely the regression these tests exist to catch. + */ +export function assertPostToolUseBlockDecision(result: HookRunResult): void { + expect(result.exitCode).toBe(0); + expect(result.parsed?.decision).toBe("block"); + expect(result.parsed?.reason).toMatch(/Blocked/i); + // The nested shape must be ABSENT, not merely ignored: copilot's shipped + // guard reads only the top level, so emitting both would leave the file + // asserting a contract no consumer actually exercises. + expect(result.parsed?.hookSpecificOutput).toBeUndefined(); +} + export function assertInstruct(result: HookRunResult): void { expect(result.exitCode).toBe(0); const output = result.parsed?.hookSpecificOutput as Record | undefined; diff --git a/__tests__/e2e/hooks/codex-integration.e2e.test.ts b/__tests__/e2e/hooks/codex-integration.e2e.test.ts index 234092c9..1cf82c28 100644 --- a/__tests__/e2e/hooks/codex-integration.e2e.test.ts +++ b/__tests__/e2e/hooks/codex-integration.e2e.test.ts @@ -15,7 +15,7 @@ import { runHook, assertAllow, assertPreToolUseDeny, - assertPostToolUseDeny, + assertPostToolUseBlockDecision, assertPermissionRequestDeny, } from "../helpers/hook-runner"; import { CodexPayloads } from "../helpers/payloads"; @@ -95,7 +95,7 @@ describe("E2E: Codex integration — hook protocol", () => { } }); - it("PostToolUse: deny emits additionalContext (Claude-compatible JSON shape)", () => { + it("PostToolUse: deny emits a top-level {decision:\"block\"}, not additionalContext", () => { const env = createCodexEnv(); try { writeConfig(env.cwd, ["sanitize-jwt"]); @@ -108,7 +108,7 @@ describe("E2E: Codex integration — hook protocol", () => { ), { homeDir: env.home, cli: "codex" }, ); - assertPostToolUseDeny(result); + assertPostToolUseBlockDecision(result); } finally { env.cleanup(); } diff --git a/__tests__/e2e/hooks/copilot-integration.e2e.test.ts b/__tests__/e2e/hooks/copilot-integration.e2e.test.ts index 3f0d9ece..1bc0ffcc 100644 --- a/__tests__/e2e/hooks/copilot-integration.e2e.test.ts +++ b/__tests__/e2e/hooks/copilot-integration.e2e.test.ts @@ -15,7 +15,7 @@ import { runHook, assertAllow, assertPreToolUseDeny, - assertPostToolUseDeny, + assertPostToolUseBlockDecision, assertCopilotStopBlock, } from "../helpers/hook-runner"; import { CopilotPayloads } from "../helpers/payloads"; @@ -93,7 +93,7 @@ describe("E2E: Copilot integration — hook protocol", () => { } }); - it("PostToolUse: deny emits additionalContext (Claude-compatible JSON shape)", () => { + it("PostToolUse: deny emits a top-level {decision:\"block\"}, not additionalContext", () => { const env = createCopilotEnv(); try { writeConfig(env.cwd, ["sanitize-jwt"]); @@ -116,7 +116,7 @@ describe("E2E: Copilot integration — hook protocol", () => { ), { homeDir: env.home, cli: "copilot" }, ); - assertPostToolUseDeny(result); + assertPostToolUseBlockDecision(result); } finally { env.cleanup(); } diff --git a/__tests__/hooks/configure-wizard.test.ts b/__tests__/hooks/configure-wizard.test.ts index a281ada1..8308add5 100644 --- a/__tests__/hooks/configure-wizard.test.ts +++ b/__tests__/hooks/configure-wizard.test.ts @@ -4,7 +4,7 @@ import { summarize, BACK, } from "../../src/hooks/tui"; import { tmpdir } from "node:os"; -import { resolve } from "node:path"; +import { resolve, dirname } from "node:path"; // The interactive prompts, the install manager, telemetry and CLI detection are // mocked so we can drive the wizard head-lessly and assert the exact side effect @@ -143,12 +143,13 @@ import { clisSupportingScope, resolvePresetSelection, reviewLines, + policyNamesLine, runConfigureWizard, maybeFirstRunConfigure, hasSeenLauncher, markLauncherSeen, } from "../../src/hooks/configure-wizard"; -import { resolvePreset, resolveEverything } from "../../src/hooks/policy-presets"; +import { resolvePreset, resolveEverything, RECOMMENDED_POLICIES } from "../../src/hooks/policy-presets"; import { INTEGRATION_TYPES, type IntegrationType } from "../../src/hooks/types"; import { getIntegration } from "../../src/hooks/integrations"; import { runPostSetupAudit } from "../../src/audit/cli"; @@ -176,6 +177,14 @@ const ttyIO = () => ({ stdin: mkTtyStdin(), stdout: mkTtyStdout() }); * `undefined` means "this step is not reached in this test". */ function drive(answers: { + /** + * Recommended-vs-customize step, asked first on every run. + * + * Defaults to "customize" when omitted, so every test written against the + * four-question wizard keeps describing the flow it was written for. A test + * that wants the one-keystroke path says so explicitly. + */ + mode?: "recommended" | "customize" | null; /** Scope step. Omitted when the run is expected to abort before it. */ target?: "user" | "project" | "both" | null; policies?: string[] | null; @@ -185,6 +194,7 @@ function drive(answers: { }) { const one = vi.mocked(selectOne); const many = vi.mocked(multiSelect); + one.mockResolvedValueOnce(("mode" in answers ? answers.mode : "customize") as never); if ("target" in answers) one.mockResolvedValueOnce(answers.target as never); if ("connect" in answers) one.mockResolvedValueOnce(answers.connect as never); if ("review" in answers) one.mockResolvedValueOnce(answers.review as never); @@ -407,6 +417,68 @@ describe("configure-wizard pure builders", () => { expect(lines).toContain("settings.json"); }); + it("reviewLines gives a taste of the policies without listing them all", () => { + // Two names say what KIND of thing these are; naming all fifteen turned a + // four-line review into a thirteen-line one, and a screen nobody reads to + // the bottom conveys less than a short one. + const lines = reviewLines({ + target: "user", + clis: ["claude"], + policies: [...RECOMMENDED_POLICIES], + cwd: "/tmp/proj", + }); + const joined = lines.join("\n"); + expect(joined).toContain("15 enabled"); + expect(joined).toContain("block-curl-pipe-sh, block-env-files +13"); + // The other thirteen are NOT on screen. + expect(joined).not.toContain("sanitize-private-key-content"); + // One line for the count, one for the taste — never a paragraph. + expect(lines.filter((l) => l.includes("block-curl-pipe-sh"))).toHaveLength(1); + }); + + it("reviewLines keeps every line inside the 80-column budget", () => { + // `writeLines` truncates with a hard cut and no ellipsis, so an over-long + // line does not visibly lose its tail — it ends mid-slug and reads as a + // policy name that does not exist. + for (const line of reviewLines({ + target: "both", + clis: ["claude"], + policies: [...RECOMMENDED_POLICIES], + cwd: "/tmp/proj", + })) { + expect(line.length, `too wide: ${line}`).toBeLessThanOrEqual(80); + } + }); + + it("the taste scales to Everything without growing", () => { + const everything = resolveEverything(); + const lines = reviewLines({ + target: "user", + clis: ["claude"], + policies: everything, + cwd: "/tmp/proj", + }).join("\n"); + expect(lines).toContain(`${everything.length} enabled`); + expect(lines).toContain(`+${everything.length - 2}`); + }); + + it("policyNamesLine drops names rather than overflowing the budget", () => { + // Degrading by dropping a name is recoverable; overflowing is not, because + // the hard cut makes the tail look like a policy name that does not exist. + const long = [ + "sanitize-private-key-content", + "sanitize-connection-strings", + "block-failproofai-commands", + ]; + for (const line of policyNamesLine(long)) { + expect(line.startsWith(" ".repeat(15))).toBe(true); + expect(line.length).toBeLessThanOrEqual(77); + } + expect(policyNamesLine([])).toEqual([]); + // A single policy needs no "+N" at all. + expect(policyNamesLine(["block-sudo"])[0].trim()).toBe("block-sudo"); + }); + it("reviewLines reports an empty policy set as a choice, not a count of zero", () => { const lines = reviewLines({ target: "user", @@ -530,6 +602,57 @@ describe("configure-wizard orchestration", () => { expect(call[8]).toEqual({ replace: true, quiet: true }); // options }); + it("Recommended asks two questions and writes the 15-policy set globally", async () => { + // The whole point of the path: scope, bundles and harnesses are never + // asked. Only mode and connect are answered here, and the run still + // applies — if the wizard had reached the policy or harness prompt it + // would hang on an unmocked multiSelect rather than pass. + drive({ mode: "recommended", connect: "local", review: "apply" }); + + const result = await runConfigureWizard(ttyIO()); + + expect(result.applied).toBe(true); + const call = vi.mocked(installHooks).mock.calls[0]; + const policies = call[0] as string[]; + expect(new Set(policies)).toEqual(new Set(RECOMMENDED_POLICIES)); + expect(call[1]).toBe("user"); // global, never the cwd's project + expect(call[7]).toEqual(["claude"]); // detected only — the mock detects claude + expect(call[8]).toEqual({ replace: true, quiet: true }); + }); + + it("Recommended never asks the policy or harness prompts", async () => { + drive({ mode: "recommended", connect: "local", review: "apply" }); + await runConfigureWizard(ttyIO()); + // `multiSelect` is the primitive both skipped steps use. + expect(multiSelect).not.toHaveBeenCalled(); + }); + + it("Recommended adds to what was already enabled, never replaces it", async () => { + // `installHooks` runs with `replace: true`, so writing the bare recommended + // list would switch OFF anything the user had enabled by hand — turning + // "give me the sensible defaults" into a REDUCTION in protection, which is + // the one direction setup must never move someone. + // + // Seeded as a real file rather than a mock: `readScopedHooksConfig` is the + // genuine implementation in this suite, and it reads user scope out of the + // HOME this file isolates. + const cfgPath = resolve(fileHome, ".failproofai", "policies-config.json"); + mkdirSync(dirname(cfgPath), { recursive: true }); + writeFileSync(cfgPath, JSON.stringify({ enabledPolicies: ["block-kubectl"] })); + try { + drive({ mode: "recommended", connect: "local", review: "apply" }); + + await runConfigureWizard(ttyIO()); + + const policies = vi.mocked(installHooks).mock.calls[0][0] as string[]; + expect(policies).toContain("block-kubectl"); // theirs, kept + expect(policies).toContain("block-rm-rf"); // ours, added + expect(new Set(policies).size).toBe(policies.length); + } finally { + rmSync(cfgPath, { force: true }); + } + }); + it("'Everything available' protects every supported CLI", async () => { drive({ target: "user", policies: ["git"], clis: ["__all_clis__"], connect: "local", review: "apply" }); // policy sources await runConfigureWizard(ttyIO()); @@ -1345,8 +1468,11 @@ describe("connect step", () => { it("lets a bad key be skipped, and still applies everything else", async () => { vi.mocked(validateIngestKey).mockResolvedValue({ ok: false, reason: "401" }); - // connect -> key, then the retry question -> skip, then review. + // mode -> customize, scope, connect -> key, then the retry question -> + // skip, then review. Queued positionally rather than through `drive()` + // because the retry prompt is conditional and has no name there. vi.mocked(selectOne) + .mockResolvedValueOnce("customize") .mockResolvedValueOnce("user") .mockResolvedValueOnce("key") .mockResolvedValueOnce("skip") diff --git a/__tests__/hooks/delivery-health.test.ts b/__tests__/hooks/delivery-health.test.ts new file mode 100644 index 00000000..d006a2ba --- /dev/null +++ b/__tests__/hooks/delivery-health.test.ts @@ -0,0 +1,157 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, utimesSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + parseParkedName, + deliveryHealth, + deliveryHealthLine, + describeAge, +} from "../../src/hooks/delivery-health"; + +let home: string; +let failed: string; + +beforeEach(() => { + home = mkdtempSync(join(tmpdir(), "fpai-delivery-health-")); + failed = join(home, ".failproofai", "state", "failed"); + mkdirSync(failed, { recursive: true }); +}); + +afterEach(() => { + rmSync(home, { recursive: true, force: true }); +}); + +/** Write a parked batch with the name the Rust uploader's `render()` produces. */ +function park(name: string, ageMs = 0): void { + const p = join(failed, name); + writeFileSync(p, '{"e":1}\n'); + if (ageMs > 0) { + const when = new Date(Date.now() - ageMs); + utimesSync(p, when, when); + } +} + +describe("hooks/delivery-health parseParkedName", () => { + // These are the exact shapes `ParkedName::render()` emits in + // crates/fpai-collect/src/uploader.rs. If that format changes, these fail — + // which is the point: the two parsers must not drift. + it("decomposes a definitively-refused batch", () => { + expect(parseParkedName("claude-2026-08-11-0.a3.c401.jsonl")).toEqual({ + base: "claude-2026-08-11-0", + attempt: 3, + clientStatus: 401, + poison: false, + }); + }); + + it("decomposes a batch parked without a client status", () => { + // No `.cNNN` — server errors exhausted their retries. The retry pass picks + // these up again, so they are NOT evidence of a bad credential. + expect(parseParkedName("claude-2026-08-11-0.a5.jsonl")).toEqual({ + base: "claude-2026-08-11-0", + attempt: 5, + clientStatus: undefined, + poison: false, + }); + }); + + it("recognises the poison suffix", () => { + const parsed = parseParkedName("claude-0.a2.c422.jsonl.poison"); + expect(parsed.poison).toBe(true); + expect(parsed.clientStatus).toBe(422); + expect(parsed.base).toBe("claude-0"); + }); + + it("leaves a non-numeric .c/.a in the base alone", () => { + // Mirrors the Rust comment: parsing rather than merely matching the prefix + // is what keeps a real spool name safe. + const parsed = parseParkedName("claude.config-0.always.jsonl"); + expect(parsed.base).toBe("claude.config-0.always"); + expect(parsed.attempt).toBe(0); + expect(parsed.clientStatus).toBeUndefined(); + }); +}); + +describe("hooks/delivery-health deliveryHealth", () => { + it("reports a machine with no failed directory as healthy", () => { + const fresh = mkdtempSync(join(tmpdir(), "fpai-delivery-health-none-")); + try { + expect(deliveryHealth(fresh)).toEqual({ + total: 0, + rejected: 0, + credentialRejected: 0, + byStatus: {}, + }); + } finally { + rmSync(fresh, { recursive: true, force: true }); + } + }); + + it("counts refused batches by status and flags credential ones", () => { + park("a-0.a1.c401.jsonl"); + park("a-1.a1.c401.jsonl"); + park("a-2.a1.c404.jsonl"); + park("a-3.a4.jsonl"); // transient: no client status + + const health = deliveryHealth(home); + expect(health.total).toBe(4); + expect(health.rejected).toBe(3); + expect(health.credentialRejected).toBe(2); + expect(health.byStatus).toEqual({ 401: 2, 404: 1 }); + }); + + it("reports the age of the oldest parked batch", () => { + park("recent.a1.c401.jsonl"); + park("old.a1.c401.jsonl", 3 * 60 * 60 * 1000); + + const health = deliveryHealth(home); + expect(health.oldestAgeMs).toBeGreaterThanOrEqual(3 * 60 * 60 * 1000 - 5_000); + }); +}); + +describe("hooks/delivery-health deliveryHealthLine", () => { + it("says nothing when only transient parks exist", () => { + // The retry pass will pick these up. Reporting them would cry wolf over a + // blip the daemon is already handling. + park("a-0.a5.jsonl"); + park("a-1.a5.jsonl"); + + const health = deliveryHealth(home); + expect(health.total).toBe(2); + expect(deliveryHealthLine(health)).toBeUndefined(); + }); + + it("names the credential and the fix when the key is being refused", () => { + park("a-0.a1.c401.jsonl"); + park("a-1.a1.c401.jsonl"); + + const line = deliveryHealthLine(deliveryHealth(home))!; + expect(line).toContain("REJECTED (401)"); + expect(line).toContain("2 batches parked"); + expect(line).toContain("--connect"); + }); + + it("does not blame the credential for a non-credential refusal", () => { + park("a-0.a1.c404.jsonl"); + + const line = deliveryHealthLine(deliveryHealth(home))!; + expect(line).toContain("REJECTED (404)"); + expect(line).not.toContain("--connect"); + }); + + it("uses the singular for one batch", () => { + park("a-0.a1.c403.jsonl"); + expect(deliveryHealthLine(deliveryHealth(home))!).toContain("1 batch parked"); + }); +}); + +describe("hooks/delivery-health describeAge", () => { + it("formats coarsely", () => { + expect(describeAge(30_000)).toBe("just now"); + expect(describeAge(5 * 60_000)).toBe("5m"); + expect(describeAge(2 * 3_600_000 + 14 * 60_000)).toBe("2h 14m"); + expect(describeAge(50 * 3_600_000)).toBe("2d 2h"); + }); +}); diff --git a/__tests__/hooks/manager.test.ts b/__tests__/hooks/manager.test.ts index 9e3fd7ec..4b80874b 100644 --- a/__tests__/hooks/manager.test.ts +++ b/__tests__/hooks/manager.test.ts @@ -550,6 +550,57 @@ describe("hooks/manager", () => { ); }); + it("replace: true rewrites the policy set without touching custom or convention policies", async () => { + // What the configure wizard's Recommended path relies on. `replace: true` + // makes the given list the whole enabled set — which is the point, since + // unticking a policy has to remove it — but it must NOT reach the two + // things a user configured separately: + // + // • customPoliciesPaths — explicit `-c` files. Wiping them would stop + // policies loading that the user never touched in this run. + // • customPoliciesEnabled — the convention-discovery flag for + // `.failproofai/policies/*.mjs`. Writing `false` here would disable + // every one of those files as a side effect of choosing a preset. + // + // Both survive because the wizard passes no `customPoliciesPath` and + // `removeCustomHooks: false`, so neither branch that rewrites them runs + // and `{...previousConfig}` carries them through. That is load-bearing + // rather than incidental, so it is asserted. + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue("{}"); + const { readScopedHooksConfig, writeScopedHooksConfig } = await import("../../src/hooks/hooks-config"); + vi.mocked(readScopedHooksConfig).mockReturnValue({ + enabledPolicies: ["block-kubectl"], + customPoliciesPaths: [resolve("/tmp/mine.js")], + customPoliciesEnabled: true, + }); + + const { installHooks } = await import("../../src/hooks/manager"); + await installHooks( + ["block-sudo", "block-rm-rf"], + "user", + undefined, + false, + "configure-wizard", + undefined, + false, + ["claude"], + { replace: true, quiet: true }, + ); + + expect(writeScopedHooksConfig).toHaveBeenCalledWith( + expect.objectContaining({ + // Replaced, as asked — block-kubectl is gone. + enabledPolicies: ["block-sudo", "block-rm-rf"], + // Preserved, untouched. + customPoliciesPaths: [resolve("/tmp/mine.js")], + customPoliciesEnabled: true, + }), + "user", + undefined, + ); + }); + it("does not duplicate a path that is already configured", async () => { vi.mocked(existsSync).mockReturnValue(true); vi.mocked(readFileSync).mockReturnValue("{}"); diff --git a/__tests__/hooks/policy-attribution.test.ts b/__tests__/hooks/policy-attribution.test.ts index a31621f3..eeeef7aa 100644 --- a/__tests__/hooks/policy-attribution.test.ts +++ b/__tests__/hooks/policy-attribution.test.ts @@ -2,7 +2,7 @@ /** * Attribution on the activity row. * - * The design doc's requirement is that Failproof Cloud can tie a decision to + * The design doc's requirement is that FailproofAI Cloud can tie a decision to * the exact rollout that produced it. Until now the only trace of a version * was a substring of a display name ("cloud/org-guard@7/…"), which nothing can * query and which re-parsing our own label would be the only way to read. diff --git a/__tests__/hooks/policy-evaluator.test.ts b/__tests__/hooks/policy-evaluator.test.ts index 390aee65..3f98809e 100644 --- a/__tests__/hooks/policy-evaluator.test.ts +++ b/__tests__/hooks/policy-evaluator.test.ts @@ -64,6 +64,58 @@ describe("hooks/policy-evaluator", () => { expect(result.reason).toBe("JWT found"); }); + // codex and copilot read a TOP-LEVEL {decision:"block", reason} at PostToolUse + // and ignore hookSpecificOutput entirely, so the additionalContext shape above + // was inert on both: emitted, logged, counted as enforcement, dropped. + // + // codex 0.147.0 live A/B: {decision:"block"} -> `hook: PostToolUse Blocked` + // and the reason replaces the tool result; hookSpecificOutput -> `hook: + // PostToolUse Completed` and the model reads the real output verbatim. + // copilot 1.0.78 app.js: both postToolUse sites gate on + // vK = t => t?.decision === "block" && typeof t.reason === "string". + it.each(["codex", "copilot"] as const)( + "PostToolUse deny on %s emits the top-level {decision:'block', reason} shape", + async (cli) => { + registerPolicy("jwt-scrub", "desc", () => ({ + decision: "deny", + reason: "JWT found", + }), { events: ["PostToolUse"] }); + + const result = await evaluatePolicies("PostToolUse", { tool_name: "Read" }, { cli }); + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); + expect(result.decision).toBe("deny"); + + const parsed = JSON.parse(result.stdout) as Record; + expect(parsed.decision).toBe("block"); + expect(parsed.reason).toBe( + "Blocked Read by failproofai because: JWT found, as per the policy configured by the user", + ); + // copilot's vK predicate fails closed unless `reason` is a string, and it + // reads no nested shape — assert both halves of that contract. + expect(typeof parsed.reason).toBe("string"); + expect(parsed.hookSpecificOutput).toBeUndefined(); + }, + ); + + it("PostToolUse deny on a CLI outside that pair still uses additionalContext", async () => { + // Guards the blast radius of the branch above: claude reads + // hookSpecificOutput here and has no top-level `decision` consumer, so + // widening the codex/copilot shape to every CLI would silently break it. + registerPolicy("jwt-scrub", "desc", () => ({ + decision: "deny", + reason: "JWT found", + }), { events: ["PostToolUse"] }); + + const result = await evaluatePolicies("PostToolUse", { tool_name: "Read" }, { cli: "claude" }); + expect(result.exitCode).toBe(0); + const parsed = JSON.parse(result.stdout) as Record; + expect(parsed.decision).toBeUndefined(); + expect( + (parsed.hookSpecificOutput as Record).additionalContext, + ).toBe("Blocked Read by failproofai because: JWT found, as per the policy configured by the user"); + }); + it("other event types use exit 2 for deny", async () => { registerPolicy("blocker", "desc", () => ({ decision: "deny", reason: "nope" }), { events: ["SessionStart"], diff --git a/__tests__/hooks/policy-presets.test.ts b/__tests__/hooks/policy-presets.test.ts index 96279e3c..80a770a5 100644 --- a/__tests__/hooks/policy-presets.test.ts +++ b/__tests__/hooks/policy-presets.test.ts @@ -4,6 +4,8 @@ import { POLICY_PRESETS, resolvePreset, resolveEverything, + RECOMMENDED_POLICIES, + defaultsMissingFromRecommended, } from "../../src/hooks/policy-presets"; describe("policy-presets", () => { @@ -59,3 +61,61 @@ describe("policy-presets", () => { expect(resolvePreset("does-not-exist")).toEqual([]); }); }); + +describe("RECOMMENDED_POLICIES", () => { + it("names 15 policies and every one of them is a real non-beta builtin", () => { + // The count is asserted because it is a product promise the wizard PRINTS + // ("15 policies · global"). Changing the set is fine; changing it without + // noticing that the screen now advertises a different number is not. + expect(RECOMMENDED_POLICIES).toHaveLength(15); + for (const name of RECOMMENDED_POLICIES) { + const policy = BUILTIN_POLICIES.find((p) => p.name === name); + expect(policy, `${name} is not a builtin policy`).toBeDefined(); + expect(policy!.beta, `${name} is beta and cannot be recommended`).toBeFalsy(); + } + }); + + it("contains no duplicates", () => { + expect(new Set(RECOMMENDED_POLICIES).size).toBe(RECOMMENDED_POLICIES.length); + }); + + it("covers every default-enabled builtin", () => { + // The drift guard. Recommended is written out by hand rather than derived, + // so the day somebody adds a new `defaultEnabled` policy it would silently + // NOT be in the recommended set — and a machine set up by pressing Enter + // would be guarded less than one set up through the policy list. The + // failure is invisible from either screen; this is the only thing looking. + expect(defaultsMissingFromRecommended()).toEqual([]); + }); + + it("excludes the policy families that must never be a default", () => { + // Each of these has a specific reason recorded next to the list: + // require-*-before-stop refuses to let the agent finish and does not fire + // at all on hermes/goose; infra blocking breaks the day job of anyone who + // runs kubectl; block-read-outside-cwd false-positives constantly. + const excludedCategories = new Set(["Workflow", "Infra Commands"]); + for (const name of RECOMMENDED_POLICIES) { + const policy = BUILTIN_POLICIES.find((p) => p.name === name)!; + expect( + excludedCategories.has(policy.category), + `${name} is in ${policy.category}, which is deliberately not recommended`, + ).toBe(false); + } + expect(RECOMMENDED_POLICIES).not.toContain("block-read-outside-cwd"); + expect(RECOMMENDED_POLICIES).not.toContain("block-work-on-main"); + }); + + it("recommends no warn-only policy", () => { + // Ten warnings is noise, and a warning nobody reads is worse than one that + // was never shown. Everything recommended actually prevents something. + expect(RECOMMENDED_POLICIES.filter((n) => n.startsWith("warn-"))).toEqual([]); + }); + + it("includes the three that were off by default and should not have been", () => { + // The gap that prompted this list: a "recommended" setup that omits + // catastrophic deletion and force-push is not recommendable. + expect(RECOMMENDED_POLICIES).toContain("block-rm-rf"); + expect(RECOMMENDED_POLICIES).toContain("block-force-push"); + expect(RECOMMENDED_POLICIES).toContain("block-secrets-write"); + }); +}); diff --git a/bin/failproofai.mjs b/bin/failproofai.mjs index c57ffac8..a0b34cc5 100755 --- a/bin/failproofai.mjs +++ b/bin/failproofai.mjs @@ -286,7 +286,7 @@ USAGE COMMANDS (no args) Launch the policy dashboard config Interactive setup — pick scope, agents & policies - --connect --token Connect to Failproof Cloud non-interactively + --connect --token Connect to FailproofAI Cloud non-interactively --machine-id Stable id for this machine --machine-label Human-readable name in the dashboard --no-transcripts Report decisions only, never transcripts @@ -1566,7 +1566,7 @@ WHAT IT DOES FAILPROOF CLOUD failproofai config --connect --token [--machine-id ] - Connect this machine to Failproof Cloud + Connect this machine to FailproofAI Cloud [--no-transcripts] decisions only, no transcripts failproofai config --disconnect Stop pulling policy and sending activity failproofai config --status Show connection, daemon and pause state diff --git a/crates/failproofaid/src/cloud_client.rs b/crates/failproofaid/src/cloud_client.rs index 74e064cc..68c8c73e 100644 --- a/crates/failproofaid/src/cloud_client.rs +++ b/crates/failproofaid/src/cloud_client.rs @@ -57,6 +57,52 @@ fn legacy_credentials_path() -> Option { .map(|home| home.join("cloud.json")) } +/// Has the operator explicitly put this machine back on OSS? +/// +/// `config --disconnect` writes `mode: "oss"` and its own comment states the +/// rule this function exists to make true: "every cloud code path keys off this +/// flag rather than off 'is a token lying around' precisely so that a +/// disconnected machine is provably silent instead of silent-by-happenstance." +/// That was true of the TypeScript CLI and false HERE — the daemon holds the +/// socket and had never read the flag. So a `mode: "oss"` machine whose +/// credential file survived (a restore, a copied home, a reinstall, a partial +/// cleanup, or simply the layout-1 `cloud.json` fallback below) went on polling +/// and shipping while the CLI reported it as disconnected. +/// +/// ONLY an explicit `"oss"` vetoes. Absent, unreadable, malformed and any other +/// value all fall through to the credential files, because `mode` postdates the +/// enrolments already in the field: reading "absent" as "oss" would silently +/// disconnect every machine enrolled by an older CLI, which is the same class of +/// silent divergence with the sign flipped. The safe direction here is that a +/// machine only goes quiet when someone said so. +fn disconnected_by_config() -> bool { + let Ok(home) = crate::paths::failproofai_home() else { + return false; + }; + let Ok(text) = std::fs::read_to_string(home.join("config.json")) else { + return false; + }; + let Ok(root) = serde_json::from_str::(&text) else { + return false; + }; + // `{"mode":{"kind":"oss"}}` — an OBJECT, not a string. + // + // This read `mode` as a string and therefore never fired: `as_str()` on an + // object is `None`, so a machine put back on OSS kept polling exactly as it + // had before the veto existed. The unit tests passed because their fixtures + // were written from the same wrong assumption as the code — a test that + // encodes the bug it is meant to catch is worth less than no test, because + // it also reports that the case is covered. + // + // The shape is `fp-config.ts`'s: `mode: { kind: config.mode }` on write + // (line 567) and `parsed.mode?.kind` on read (line 423). There is no flat + // form to fall back to — the TS has never written one. + root.get("mode") + .and_then(|m| m.get("kind")) + .and_then(|k| k.as_str()) + == Some("oss") +} + /// The `cloud` object of `credentials.json`. Snake_case keys, because that is /// what `fp-config.ts`'s `writeCredentials` emits. #[derive(serde::Deserialize)] @@ -137,6 +183,21 @@ impl CloudClient { /// the daemon never looked at, and the daemon logged "cloud-managed policy /// polling disabled" as though the machine had simply never enrolled. pub fn from_file() -> Result, String> { + // `mode: "oss"` outranks every credential file below it. Checked HERE + // rather than at the call site because `from_file` has three exits (the + // override, `credentials.json`, and the layout-1 fallback) and a veto + // that guards only some of them is not a veto. + // + // Deliberately NOT applied to `from_env()`: `FAILPROOFAI_CLOUD_URL` is + // an explicit, per-process act by whoever launched the daemon, and the + // env path exists so CI, containers and tests work with no files at all. + // Letting a file on disk veto that would break exactly the callers the + // env path was added for, and an operator who exports the variable has + // said what they want more recently than the config did. + if disconnected_by_config() { + return Ok(None); + } + // An explicitly-named file is the whole configuration: if it is absent, // this machine is not enrolled. Falling through to the default location // would quietly enrol it against a DIFFERENT credential than the one the @@ -252,13 +313,38 @@ impl CloudClient { }) } - pub fn desired_state(&self) -> Result { + /// `applied` is the deployment this machine is ACTUALLY enforcing right now, + /// read from `active.json`. + /// + /// The server has never been able to tell "assigned" from "applied". It + /// infers delivery by comparing the machine's last poll against the + /// deployment's `updated_at` — so a machine that polled and then failed to + /// materialise the artifacts reads as delivered, and the dashboard shows + /// `applied` for a deployment that is provably not in force. The machine has + /// always known the true answer and had no way to say it; this is that way. + /// + /// It rides the poll that already happens every 30s rather than a new + /// endpoint or a second connection, so it costs one query parameter and no + /// extra request. Additive on purpose: a server that does not read the + /// parameter is unaffected, which is the ordering #590 asks for — the daemon + /// may ship before the server without a coordinated release. + /// + /// `None` when nothing is active yet (never polled successfully, or the + /// manifest is unreadable). The parameter is then OMITTED rather than sent + /// as 0: a machine that cannot say what it is enforcing must not be recorded + /// as enforcing deployment zero, and absent has to stay distinguishable from + /// "reported nothing" on the far side. + pub fn desired_state(&self, applied: Option) -> Result { let mut url = self .base_url .join("enforcement/v1/desired-state") .map_err(|err| format!("failed to build desired-state URL: {err}"))?; url.query_pairs_mut() .append_pair("machineId", &self.machine_id); + if let Some(deployment) = applied { + url.query_pairs_mut() + .append_pair("appliedDeployment", &deployment.to_string()); + } self.client .get(url) .bearer_auth(&self.token) @@ -388,7 +474,23 @@ pub fn spawn_maintenance( } fn poll_once(store: &PolicyStore, cloud: &CloudClient) { - match cloud.desired_state() { + // Read BEFORE the request, so what we report is what was in force when we + // asked. Reading after would race this poll's own reconcile and could claim + // a deployment the server is about to be told about anyway — reporting the + // future rather than the present. + // + // An unreadable manifest reports nothing rather than guessing: `read_active` + // already distinguishes "no deployment" from "cannot tell", and collapsing + // the second into the first is how a machine ends up recorded as enforcing + // something it is not. + let applied = match store.read_active() { + Ok(active) => active.map(|a| a.deployment), + Err(err) => { + eprintln!("[failproofaid] could not read the active deployment to report it: {err}"); + None + } + }; + match cloud.desired_state(applied) { Ok(desired) => { match store.reconcile(&desired, &|policy: &DesiredPolicy| cloud.artifact(policy)) { Ok(outcome) @@ -470,6 +572,17 @@ mod tests { } unsafe { std::env::set_var("FAILPROOFAI_CLOUD_CREDENTIALS", &path); + // Point HOME at the scratch dir too. + // + // Without this the suite reads the DEVELOPER'S real + // ~/.failproofai/config.json, and `from_file` consults it now that + // the `mode` veto works. Five tests here failed the moment the veto + // started firing — not because the veto was wrong, but because a + // machine that had run `--disconnect` (or, as here, was simply set + // up in OSS mode) made them fail while the same commit passed on a + // machine that had not. A unit test whose result depends on the + // config of the laptop running it is not testing what it says. + std::env::set_var("FAILPROOFAI_HOME", &dir); std::env::remove_var("FAILPROOFAI_CLOUD_URL"); std::env::remove_var("FAILPROOFAI_CLOUD_TOKEN"); std::env::remove_var("FAILPROOFAI_MACHINE_ID"); @@ -605,7 +718,7 @@ mod tests { ) .expect("client"); let err = client - .desired_state() + .desired_state(None) .expect_err("a v1 response must be refused"); assert!( @@ -615,6 +728,59 @@ mod tests { server.join().unwrap(); } + /// The machine's own answer has to reach the wire, and "cannot say" has to + /// stay distinguishable from "deployment 0" once it gets there. + #[test] + fn reports_the_applied_deployment_on_the_poll_it_already_makes() { + for (applied, expected) in [(Some(7_u64), true), (None, false)] { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let address = listener.local_addr().unwrap(); + let captured = Arc::new(std::sync::Mutex::new(String::new())); + let sink = captured.clone(); + let server = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().unwrap(); + let mut request = [0_u8; 4096]; + let read = stream.read(&mut request).unwrap(); + *sink.lock().unwrap() = String::from_utf8_lossy(&request[..read]).to_string(); + let body = br#"{"schemaVersion":2,"deployment":7,"policies":[]}"#; + write!( + stream, + "HTTP/1.1 200 OK\r\nContent-Length: {}\r\nContent-Type: application/json\r\nConnection: close\r\n\r\n", + body.len() + ) + .unwrap(); + stream.write_all(body).unwrap(); + }); + + let cloud = CloudClient::new( + &format!("http://{address}"), + "test-token".into(), + "machine-1".into(), + ) + .unwrap(); + cloud.desired_state(applied).unwrap(); + server.join().unwrap(); + + let request = captured.lock().unwrap().clone(); + assert!( + request.contains("machineId=machine-1"), + "the existing parameter must survive: {request}" + ); + if expected { + assert!( + request.contains("appliedDeployment=7"), + "the applied deployment must reach the server: {request}" + ); + } else { + assert!( + !request.contains("appliedDeployment"), + "a machine that cannot say what it is enforcing must OMIT the \ + parameter, not report deployment 0: {request}" + ); + } + } + } + #[test] fn fetches_desired_state_and_artifact_into_the_store() { let artifact = b"export default 'managed';\n".to_vec(); @@ -665,7 +831,7 @@ mod tests { "machine-1".into(), ) .unwrap(); - let desired = cloud.desired_state().unwrap(); + let desired = cloud.desired_state(None).unwrap(); let root = std::env::temp_dir().join(format!("failproofaid-http-test-{}", std::process::id())); let _ = fs::remove_dir_all(&root); @@ -811,6 +977,102 @@ mod tests { unsafe { std::env::remove_var("FAILPROOFAI_HOME") }; } + #[test] + fn mode_oss_vetoes_a_surviving_credentials_file() { + // The reported bug: switch back to OSS, and the machine keeps talking to + // the cloud because the credential outlived the decision to leave. + let _lock = lock_env(); + let _guard = with_home(&[ + ("credentials.json", FILE_CREDS), + ("config.json", r#"{"mode":{"kind":"oss"}}"#), + ]); + assert!(CloudClient::from_file().unwrap().is_none()); + unsafe { std::env::remove_var("FAILPROOFAI_HOME") }; + } + + #[test] + fn mode_oss_vetoes_the_layout_1_fallback_too() { + // The fallback is the easiest of the three exits to leave unguarded, and + // the one most likely to be the file that survived a cleanup. + let _lock = lock_env(); + let _guard = with_home(&[ + ("cloud.json", GOOD), + ("config.json", r#"{"mode":{"kind":"oss"}}"#), + ]); + assert!(CloudClient::from_file().unwrap().is_none()); + unsafe { std::env::remove_var("FAILPROOFAI_HOME") }; + } + + #[test] + fn mode_cloud_still_enrols() { + let _lock = lock_env(); + let _guard = with_home(&[ + ("credentials.json", FILE_CREDS), + ("config.json", r#"{"mode":{"kind":"cloud"}}"#), + ]); + let client = CloudClient::from_file().unwrap().expect("enrolled"); + assert_eq!(client.machine_id, "m-json"); + unsafe { std::env::remove_var("FAILPROOFAI_HOME") }; + } + + #[test] + fn an_absent_or_unreadable_mode_does_not_disconnect_anyone() { + // `mode` postdates the enrolments already in the field. Reading absent, + // malformed or unexpected as "oss" would silently disconnect every + // machine enrolled by an older CLI — the same silent divergence this + // veto exists to close, with the sign flipped. + let _lock = lock_env(); + for config in [ + None, + Some(r#"{}"#), + Some(r#"{ not json"#), + Some(r#"{"mode":"OSS"}"#), + Some(r#"{"mode":true}"#), + // The shape this function USED to read. The TS has never written a + // flat string — `fp-config.ts` writes `mode: { kind }` — so a bare + // string is malformed for this schema and must not veto. Kept as a + // fixture because it is precisely what the original fixtures said, + // which is how the veto shipped never firing. + Some(r#"{"mode":"oss"}"#), + // Nested but not the value we act on. + Some(r#"{"mode":{"kind":"cloud"}}"#), + Some(r#"{"mode":{}}"#), + ] { + let mut files = vec![("credentials.json", FILE_CREDS)]; + if let Some(c) = config { + files.push(("config.json", c)); + } + let _guard = with_home(&files); + assert!( + CloudClient::from_file().unwrap().is_some(), + "config {config:?} must not disconnect a machine nobody disconnected", + ); + } + unsafe { std::env::remove_var("FAILPROOFAI_HOME") }; + } + + #[test] + fn env_configuration_outranks_mode_oss() { + // `FAILPROOFAI_CLOUD_URL` is an explicit act by whoever launched the + // daemon, and the env path exists so CI/containers work with no files. + // A file on disk must not veto it. + let _lock = lock_env(); + let _guard = with_home(&[("config.json", r#"{"mode":{"kind":"oss"}}"#)]); + unsafe { + std::env::set_var("FAILPROOFAI_CLOUD_URL", "https://cloud.example"); + std::env::set_var("FAILPROOFAI_CLOUD_TOKEN", "t"); + std::env::set_var("FAILPROOFAI_MACHINE_ID", "m-env"); + } + let client = CloudClient::from_env_or_file().unwrap().expect("enrolled"); + assert_eq!(client.machine_id, "m-env"); + unsafe { + std::env::remove_var("FAILPROOFAI_CLOUD_URL"); + std::env::remove_var("FAILPROOFAI_CLOUD_TOKEN"); + std::env::remove_var("FAILPROOFAI_MACHINE_ID"); + std::env::remove_var("FAILPROOFAI_HOME"); + } + } + #[test] fn prefers_credentials_json_when_both_exist() { // Mid-migration both are on disk, and credentials.json is the current one. diff --git a/crates/failproofaid/src/main.rs b/crates/failproofaid/src/main.rs index 7930e10a..51a3a456 100644 --- a/crates/failproofaid/src/main.rs +++ b/crates/failproofaid/src/main.rs @@ -1086,6 +1086,11 @@ fn collector_tasks() -> Vec { for (i, db) in hermes_dbs.iter().enumerate() { let profile = profile_dir_name(db, i); let state = cursors.join("hermes").join(&profile); + // Bound outside the call: the id is borrowed for the whole + // `sqlite_source` invocation, and a temporary would be dropped at + // the end of the argument expression. + let hermes_agent_id = + hermes::agent_id_for(hermes::DEFAULT_AGENT_ID, hermes::profile_of(db).as_deref()); // ...and its own health key, for the same reason it gets its own // cursor directory. Every profile reporting under the bare string // "hermes" made two profiles overwrite each other's record five @@ -1097,7 +1102,15 @@ fn collector_tasks() -> Vec { "hermes", hermes::FORMAT, db.clone(), - hermes::DEFAULT_AGENT_ID, + // The DATABASE's id, not a bare constant. Every profile used to + // pass `hermes` and let the transform re-derive a name from each + // session's own mutable columns — which is what split one + // session across two agent ids. `agent_id_for` keys on the path: + // the root keeps the bare `hermes` every deployment already + // ships under, and `profiles//state.db` becomes + // `hermes-`, matching the standalone collector so a + // machine migrating off it is not renamed. + &hermes_agent_id, &spool, state, &env, @@ -1198,7 +1211,7 @@ fn file_source( format: fpai_collect::filetail::Format, roots: Vec, extra: &[fpai_collect::ExtraPath], - default_agent_id: &'static str, + default_agent_id: &str, spool_dir: &std::path::Path, cursor_root: &std::path::Path, environment: &str, @@ -1258,7 +1271,7 @@ fn file_source_instance( label: Option, state_dir: std::path::PathBuf, health_key: Option, - default_agent_id: &'static str, + default_agent_id: &str, spool_dir: &std::path::Path, environment: &str, machine_id: Option<&str>, @@ -1269,6 +1282,11 @@ fn file_source_instance( let environment = environment.to_string(); let machine_id = machine_id.map(str::to_string); let user = user.map(str::to_string); + // Owned before the task closure captures it. This was `&'static str` while + // every caller passed a literal; a per-database id is computed at startup, + // so it has to be cloned here rather than borrowed into a closure that + // outlives this frame. + let default_agent_id = default_agent_id.to_string(); // The task name carries the label so `/workflows`-style task listings and // the supervisor's own logs name the instance, not just the source. let task_name: String = match &label { @@ -1327,7 +1345,7 @@ fn sqlite_harness( format: fpai_collect::sqlitepoll::SqliteFormat, db_path: std::path::PathBuf, extra: &[fpai_collect::ExtraPath], - default_agent_id: &'static str, + default_agent_id: &str, spool_dir: &std::path::Path, cursor_root: &std::path::Path, environment: &str, @@ -1376,7 +1394,7 @@ fn sqlite_source( name: &'static str, format: fpai_collect::sqlitepoll::SqliteFormat, db_path: std::path::PathBuf, - default_agent_id: &'static str, + default_agent_id: &str, spool_dir: &std::path::Path, state_dir: std::path::PathBuf, environment: &str, @@ -1394,6 +1412,11 @@ fn sqlite_source( let environment = environment.to_string(); let machine_id = machine_id.map(str::to_string); let user = user.map(str::to_string); + // Owned before the task closure captures it. This was `&'static str` while + // every caller passed a literal; a per-database id is computed at startup, + // so it has to be cloned here rather than borrowed into a closure that + // outlives this frame. + let default_agent_id = default_agent_id.to_string(); let task_name: String = match &label { Some(l) => format!("{name}:{l}"), None => name.to_string(), diff --git a/crates/fpai-collect/src/sources/hermes/mod.rs b/crates/fpai-collect/src/sources/hermes/mod.rs index d5373f88..95314c6b 100644 --- a/crates/fpai-collect/src/sources/hermes/mod.rs +++ b/crates/fpai-collect/src/sources/hermes/mod.rs @@ -294,6 +294,59 @@ pub fn default_db_paths() -> Vec { /// [`default_db_paths`] against an explicit root. Split out so it is testable /// without mutating the process environment. +/// The agent id for one Hermes database: the base for the root, `-` +/// for a named profile. +/// +/// IDENTITY COMES FROM WHICH DATABASE THIS IS, and deliberately not from +/// anything inside it. Sessions were previously named from their own `cwd` or +/// `source` columns, which Hermes rewrites throughout a run (`hermes_state.py` +/// carries ~20 `UPDATE sessions SET …`). Because the id was re-derived on every +/// poll, a session whose `cwd` was written or cleared between two polls had its +/// rows split across two agent ids — observed live: one session under +/// `hermes-kratos` and `hermes-telegram`, another under `hermes-cron` and the +/// bare fallback. A file path cannot change under us mid-poll, so this is stable +/// by construction rather than by remembering anything. +/// +/// It also keeps the poll function PURE, which the format contract requires: +/// re-reading the same rows must produce the same bytes or the server's +/// content-hash dedup stores them twice instead of collapsing them. Deriving +/// from a mutable column broke that; deriving from the request does not. +/// +/// Nothing is lost by moving `source` and `cwd` out of the NAME — both are +/// already emitted in the payload as `hermes_source` and `hermes_cwd`, so +/// "show me Telegram traffic" or "group by project" stay answerable as filters, +/// which is what they should have been. They are attributes of a session, not +/// the identity of an agent. +/// +/// The root profile stays on the bare base rather than `hermes-default`: it is +/// the id every existing deployment already ships under, and prefixing it would +/// split each one's history across two agent ids for no gain. This matches the +/// standalone collector's `agent_id_for`, so machines migrating from it keep +/// their names. +pub fn agent_id_for(base: &str, profile: Option<&str>) -> String { + match profile { + None => base.to_string(), + Some(name) => format!("{base}-{name}"), + } +} + +/// The profile name for a database returned by [`db_paths_under`], or `None` +/// for the root. +/// +/// Keyed on the path shape that function produces — `/state.db` for the +/// root and `/profiles//state.db` for the rest — rather than on an +/// index, so a caller cannot silently mis-name a profile by iterating in a +/// different order. +pub fn profile_of(db: &Path) -> Option { + let parent = db.parent()?; + let name = parent.file_name()?.to_str()?; + let is_profile = parent + .parent() + .and_then(|p| p.file_name()) + .is_some_and(|d| d == PROFILES_DIR); + is_profile.then(|| name.to_string()) +} + pub fn db_paths_under(root: &Path) -> Vec { let mut out = vec![root.join(STATE_DB)]; let Ok(entries) = std::fs::read_dir(root.join(PROFILES_DIR)) else { diff --git a/crates/fpai-collect/src/sources/hermes/transform.rs b/crates/fpai-collect/src/sources/hermes/transform.rs index 9c3b0452..f863f051 100644 --- a/crates/fpai-collect/src/sources/hermes/transform.rs +++ b/crates/fpai-collect/src/sources/hermes/transform.rs @@ -144,49 +144,42 @@ pub fn sanitize_id_part(s: &str) -> String { .collect() } -/// Last path component of a working directory, sanitized. -fn project_name(cwd: &str) -> Option { - let last = cwd - .trim_end_matches('/') - .rsplit('/') - .find(|p| !p.is_empty())?; - let cleaned = sanitize_id_part(last); - (!cleaned.is_empty()).then_some(cleaned) -} - -/// The agent id a session's events are filed under. +/// The agent id a session's events are filed under: the DATABASE's, always. /// -/// Hermes has BOTH shapes and each needs its own answer: +/// Hermes has two session shapes — a CLI session with a real `cwd`, and a +/// gateway session (Slack, Telegram, cron) whose `cwd` is NULL — and this used +/// to give each its own agent id, derived from those columns. That is what broke: +/// Hermes rewrites `sessions` throughout a run, the id was re-derived on every +/// poll, and a session split across two agent ids as soon as a column changed +/// between passes. /// -/// * A **CLI** session has a real `cwd` (5/5 in the probe capture, alongside -/// `git_branch` and `git_repo_root`), so it groups by project exactly like -/// Claude and the hook source do — which is the whole point: hook events and -/// session events for one run must land under a single agent rather than two -/// that look unrelated. -/// * A **gateway** session (Slack, Telegram, cron) genuinely has NULL `cwd`. -/// There is no project to group by, so it falls back to the transport in -/// `source`. Grouping every gateway session under one bare `hermes` would -/// merge Slack and Telegram traffic into an agent nobody can act on. +/// Both shapes are still distinguishable, as `hermes_source` and `hermes_cwd` +/// on every event — which is where "which transport" and "which project" belong: +/// filters over one agent's sessions, not separate agents. /// -/// `fallback` — the collector's configured default — is used only when the -/// session row itself is missing, which means a message referencing a deleted -/// session. -pub fn agent_id(meta: Option<&SessionMeta>, fallback: &str) -> String { - let Some(meta) = meta else { - return fallback.to_string(); - }; - if let Some(project) = meta.cwd.as_deref().and_then(project_name) { - return format!("hermes-{project}"); - } - if let Some(source) = meta - .source - .as_deref() - .map(sanitize_id_part) - .filter(|s| !s.is_empty()) - { - return format!("hermes-{source}"); - } - fallback.to_string() +pub fn agent_id(_meta: Option<&SessionMeta>, profile_agent_id: &str) -> String { + // The database's id, unconditionally. + // + // This used to prefer the session's own `cwd` (as a project name) and then + // its `source`, falling back to the caller's id. Both of those columns are + // rewritten by Hermes DURING a run, and the id was re-derived on every poll + // — so a session split across two agent ids the moment one of them changed + // between polls. Live evidence: `20260812_133702_31ca19f0` arrived under + // both `hermes-kratos` and `hermes-telegram`, and `cron_bec543608ab8_…` + // under both `hermes-cron` and the bare fallback, from a single collector. + // + // The caller's id is derived from the database path (`agent_id_for`), which + // cannot change while a poll is running. That makes identity stable by + // construction and keeps this function pure with respect to + // `(connection contents, request)` — the property the format contract + // requires so re-read rows dedup instead of duplicating. + // + // `meta` is still taken so the signature does not churn at every call site, + // and because the session's attributes remain worth carrying: they are + // emitted in the payload as `hermes_source` and `hermes_cwd`, which is where + // "which channel" and "which project" belong — as filters over an agent's + // sessions, not as separate agents. + profile_agent_id.to_string() } /// The row's timestamp as epoch milliseconds, or `None` when it is implausible. diff --git a/crates/fpai-collect/tests/hermes_source.rs b/crates/fpai-collect/tests/hermes_source.rs index e76b3293..8a1ad02d 100644 --- a/crates/fpai-collect/tests/hermes_source.rs +++ b/crates/fpai-collect/tests/hermes_source.rs @@ -540,10 +540,14 @@ fn a_non_zero_exit_code_is_not_treated_as_a_tool_error() { // ── agent id: both session shapes ──────────────────────────────────────── #[test] -fn a_cli_session_is_grouped_by_its_working_directory() { - // `cwd` IS populated for CLI sessions (5/5 on the probe), so they group by - // project exactly like the hook source does — which is what makes hook - // events and session events for one run land under a single agent. +fn a_cli_session_keeps_the_databases_agent_id_and_carries_its_cwd() { + // `cwd` IS populated for CLI sessions (5/5 on the probe) and is still + // shipped — as a FIELD. It is no longer the agent's name. + // + // Naming the agent from it meant identity was re-derived from a column + // Hermes rewrites mid-run, on every poll, so one session split across two + // agent ids the moment it changed. Grouping by project stays available as a + // filter over `hermes_cwd`, which is what it should have been. let dir = tmpdir("cwd"); let (db, conn) = make_db(&dir); insert_session( @@ -561,7 +565,7 @@ fn a_cli_session_is_grouped_by_its_working_directory() { let ev = poll_all(&db); assert!(!ev.is_empty()); for e in &ev { - assert_eq!(e["agent_id"], "hermes-openclaw-local"); + assert_eq!(e["agent_id"], "hermes"); assert_eq!(e["hermes_cwd"], "/home/u/src/openclaw-local"); } @@ -569,10 +573,11 @@ fn a_cli_session_is_grouped_by_its_working_directory() { } #[test] -fn a_gateway_session_with_no_cwd_falls_back_to_its_source() { - // Slack/Telegram sessions genuinely have NULL cwd. Falling all the way back - // to a bare `hermes` would merge every transport into one agent nobody can - // act on, so the transport is the grouping axis instead. +fn a_gateway_session_keeps_the_databases_agent_id_and_carries_its_source() { + // Slack/Telegram sessions genuinely have NULL cwd, and their transport is + // still shipped as `hermes_source` — again as a field, not as the name. + // Transport remains the grouping axis; it is applied by filtering rather + // than by fragmenting one database into several agents. let dir = tmpdir("gateway"); let (db, conn) = make_db(&dir); insert_session( @@ -590,7 +595,7 @@ fn a_gateway_session_with_no_cwd_falls_back_to_its_source() { let ev = poll_all(&db); assert!(!ev.is_empty()); for e in &ev { - assert_eq!(e["agent_id"], "hermes-slack"); + assert_eq!(e["agent_id"], "hermes"); assert_eq!(e["hermes_source"], "slack"); assert!( e.get("hermes_cwd").is_none(), @@ -881,7 +886,7 @@ fn the_fts_shadow_tables_are_populated_but_never_read() { } #[test] -fn several_sessions_in_one_batch_keep_their_own_agent_ids() { +fn several_sessions_in_one_batch_share_the_databases_agent_id() { // One database holds every session on the machine, CLI and gateway alike. // A poll that resolved the session once and reused it would file a Slack // turn under the last CLI project it happened to see. @@ -912,8 +917,77 @@ fn several_sessions_in_one_batch_keep_their_own_agent_ids() { let ev = poll_all(&db); let reqs = of_type(&ev, "model_request"); - assert_eq!(reqs[0]["agent_id"], "hermes-work"); - assert_eq!(reqs[1]["agent_id"], "hermes-telegram"); + // One database, one agent — whatever the sessions inside it are doing. + assert_eq!(reqs[0]["agent_id"], "hermes"); + assert_eq!(reqs[1]["agent_id"], "hermes"); + // The distinction between them is preserved where it belongs: on the rows. + assert_eq!(reqs[0]["hermes_cwd"], "/work"); + assert_eq!(reqs[1]["hermes_source"], "telegram"); + + fs::remove_dir_all(&dir).ok(); +} + +#[test] +fn a_session_keeps_one_agent_id_when_hermes_rewrites_its_cwd_mid_run() { + // THE REGRESSION THIS EXISTS FOR. + // + // Hermes rewrites `sessions` throughout a run (`hermes_state.py` carries + // ~20 `UPDATE sessions SET …`), and the agent id used to be re-derived from + // those columns on EVERY poll. So a session whose `cwd` was written or + // cleared between two polls had its rows split across two agent ids, from a + // single collector. Observed live on a customer org: one session arrived + // under both `hermes-kratos` and `hermes-telegram`, another under + // `hermes-cron` and the bare fallback. + // + // Polling twice with the column changed in between is the only way to catch + // it — every existing test polls once, which is why a bug that had been + // shipping was invisible to a green suite. + let dir = tmpdir("stable-id"); + let (db, conn) = make_db(&dir); + insert_session( + &conn, + CLI_SESSION, + "telegram", + None, + 1_785_744_251.6, + None, + None, + None, + ); + user(&conn, CLI_SESSION, "first turn", 1_785_744_251.7); + + let before = poll_at(&db, 0, 1000); + let watermark = before.watermark; + assert!(!before.events.is_empty()); + + // Hermes sets a working directory partway through the run. + conn.execute( + "UPDATE sessions SET cwd = ?1 WHERE id = ?2", + rusqlite::params!["/home/u/src/kratos", CLI_SESSION], + ) + .unwrap(); + user(&conn, CLI_SESSION, "second turn", 1_785_744_252.9); + + let after = poll_at(&db, watermark, 1000); + assert!( + !after.events.is_empty(), + "the second turn must be collected" + ); + + for e in before.events.iter().chain(after.events.iter()) { + assert_eq!( + e["agent_id"], "hermes", + "one session must never span two agent ids: {e:?}" + ); + } + // The new cwd is still REPORTED — it just is not the agent's name. + assert!( + after + .events + .iter() + .any(|e| e["hermes_cwd"] == "/home/u/src/kratos"), + "the updated cwd must still reach the payload" + ); fs::remove_dir_all(&dir).ok(); } @@ -1066,7 +1140,7 @@ async fn a_session_is_spooled_once_and_a_resumed_run_ships_nothing() { } for e in &ev { assert_eq!(e["session_id"], CLI_SESSION); - assert_eq!(e["agent_id"], "hermes-work"); + assert_eq!(e["agent_id"], "hermes"); assert_eq!(e["environment"], "local"); } diff --git a/package.json b/package.json index 36bd412c..2021b064 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "failproofai", - "version": "1.0.0-beta.22", + "version": "1.0.0", "description": "The easiest way to manage policies that keep your AI agents reliable, on-task, and running autonomously — for Claude Code & the Agents SDK", "bin": { "failproofai": "./dist/cli.mjs", diff --git a/src/audit/audit-schedule.ts b/src/audit/audit-schedule.ts index 9a671261..da67b261 100644 --- a/src/audit/audit-schedule.ts +++ b/src/audit/audit-schedule.ts @@ -35,7 +35,7 @@ * drive it; the daemon derives and writes this file. * - `next-audit.json` = an **email nudge to a signed-in human** ("come back * and audit again"), set from the /audit "come back better" section, - * delivered by Failproof Cloud over SES via the api-server. It runs nothing + * delivered by FailproofAI Cloud over SES via the api-server. It runs nothing * locally; it is re-engagement, not a schedule. * * Collapsing them would merge "the box scans on a timer" with "email this person diff --git a/src/hooks/cloud-connection.ts b/src/hooks/cloud-connection.ts index 52fedbde..9f933036 100644 --- a/src/hooks/cloud-connection.ts +++ b/src/hooks/cloud-connection.ts @@ -1,5 +1,5 @@ /** - * One connection to Failproof Cloud, with two capabilities. + * One connection to FailproofAI Cloud, with two capabilities. * * # Why this exists * diff --git a/src/hooks/cloud-enrollment-cli.ts b/src/hooks/cloud-enrollment-cli.ts index 6e7e0841..44ed4050 100644 --- a/src/hooks/cloud-enrollment-cli.ts +++ b/src/hooks/cloud-enrollment-cli.ts @@ -20,6 +20,7 @@ import { daemonVersionSkew, } from "./daemon-service"; import { clearActiveCloudManagedPolicies } from "./cloud-managed-policies"; +import { deliveryHealth, deliveryHealthLine } from "./delivery-health"; import { readVersionFile, readCredentials } from "./fp-config"; import { version as cliVersion } from "../../package.json"; import { daemonSocketPresent } from "./daemon-client"; @@ -281,7 +282,7 @@ export async function runRenameCommand( return { exitCode: 1, lines: [ - "This machine is not connected to Failproof Cloud, so it has no name to change.", + "This machine is not connected to FailproofAI Cloud, so it has no name to change.", "Connect it first: failproofai config --connect --token ", ], }; @@ -336,11 +337,11 @@ export function runDisconnectCommand(): CommandResult { updateConfig({ mode: "oss" }); if (!removed && !existing && !hadIngest && !stoppedManaged) { - return { exitCode: 0, lines: ["This machine is not connected to Failproof Cloud."] }; + return { exitCode: 0, lines: ["This machine is not connected to FailproofAI Cloud."] }; } const lines = [ - "Disconnected from Failproof Cloud.", + "Disconnected from FailproofAI Cloud.", "", stoppedManaged ? " Cloud-managed policies stop being enforced and stop being refreshed.\n" + @@ -428,7 +429,21 @@ export function connectionStatusLines( if (orgLine) lines.push(` Org ${orgLine}`); if (ingest) { - lines.push(` Dashboard sending hook activity to ${ingest.url}.`); + // Everything above this line is read from the credential FILE, which + // records what was true at `--connect` time and is never revisited. A key + // that has since been revoked, expired, or had its org disabled leaves that + // file byte-for-byte correct while nothing arrives — which is how a machine + // reported "connected" for twenty minutes with 26 refused batches on disk + // (see `crates/failproofaid/src/main.rs`). The collector's own record of + // what the server actually said is the only thing here that describes NOW, + // so it overrides the cheerful line rather than being appended after it. + const health = deliveryHealth(); + const rejection = deliveryHealthLine(health); + if (rejection) { + lines.push(` Dashboard ${rejection}`); + } else { + lines.push(` Dashboard sending hook activity to ${ingest.url}.`); + } } else { lines.push(` Dashboard NOT sending — nothing from this machine appears in the dashboard.`); lines.push(` Re-run: failproofai config --connect ${creds!.url} --token `); diff --git a/src/hooks/cloud-enrollment.ts b/src/hooks/cloud-enrollment.ts index 60841d7d..b3efe789 100644 --- a/src/hooks/cloud-enrollment.ts +++ b/src/hooks/cloud-enrollment.ts @@ -1,5 +1,5 @@ /** - * Connecting a machine to Failproof Cloud. + * Connecting a machine to FailproofAI Cloud. * * The credential lives in a file the daemon reads, NOT in the service unit. * `daemon-service.ts` installs that unit at mode 0644 into @@ -278,7 +278,7 @@ export async function verifyCloudCredentials(creds: CloudCredentials): Promise 0 ? `${shown.join(", ")} +${rest}` : shown.join(", "); + if (text.length <= budget) return [`${POLICY_LIST_INDENT}${text}`]; + } + return []; +} + /** * Name the bundles rather than counting the policies inside them. * @@ -643,6 +677,12 @@ export function reviewLines(state: { ? " Policies : none enabled (add later: failproofai policies --install)" : ` Policies : ${policies.length} enabled`, ); + // A taste of what they are, under the count. "15 enabled" alone is a number + // the user cannot check and, on the recommended path, did not choose; two + // names say what kind of thing it is without turning the review into a page. + if (policies.length > 0) { + lines.push(...policyNamesLine([...policies].sort())); + } if (installDaemon && isDaemonSupportedPlatform()) { lines.push( ` Daemon : failproofaid, installed as a system service running as you`, @@ -1006,14 +1046,20 @@ export async function runConfigureWizard(io: WizardIO = {}): Promise clisSupportingScope("user").includes(id)); + const mode = await selectOne<"recommended" | "customize">({ + message: "Set up failproofai", + choices: [ + { + label: "Recommended", + value: "recommended", + hint: recommendedClis.length + ? `${recommendedClis.length} detected ${recommendedClis.length === 1 ? "CLI" : "CLIs"} · ${RECOMMENDED_POLICIES.length} policies · global` + : `${RECOMMENDED_POLICIES.length} policies · global`, + }, + { + label: "Customize", + value: "customize", + hint: "choose scope, policies and harnesses", + }, + ], + stdin, + stdout, + }); + if (mode === null) return cancel(); + // 1 — Where? Inferred from cwd, then confirmed. // // Running from inside a project and running from a home directory are two @@ -1067,7 +1153,13 @@ export async function runConfigureWizard(io: WizardIO = {}): Promise({ - message: "Connect this machine to Failproof Cloud?", + message: "Connect this machine to FailproofAI Cloud?", + // The hint says what you GET; the body says what LEAVES. + // + // Both have to be here and they are different jobs. "sends decisions + + // transcripts" described the plumbing, which reads like a data-collection + // notice rather than a feature — the reason anyone connects is to see + // what their agents did, so the option says that. + // + // The disclosure stays in the BODY rather than moving into the hint, + // which was the original design and is right: this screen is the only + // place it is ever made. `describeOutcome` prints "hook activity" after + // connecting and never mentions transcripts, so a person who does not + // read it here does not read it at all. One line, not three, and the + // specific nouns survive — "sessions" alone would not tell anyone their + // file contents leave the machine. + // + // The get-started page, not the dashboard host. + // + // `app.befailproof.ai` is only useful to somebody who already has an + // account, and this line addresses the person who has none — new to the + // product, so no org either, not merely missing a key. The get-started + // route walks them through creating both, so it answers the question + // actually being asked. Naming the product rather than the artefact is + // what makes it self-selecting: "No key?" reads as an error state to + // someone who simply has not signed up yet. + // + // A marketing route rather than a dashboard one is also the safer thing + // to hard-code here — it is the URL the site is expected to keep stable, + // where an in-app path can be reorganised without anyone thinking to + // update a string compiled into a CLI. + // + // The scope is gone from this screen: it is enforced at paste time, where + // a wrong key is actually caught. body: [ - " Connecting reports this machine's policy decisions AND full session", - " transcripts — prompts, file contents and command output — to your", - " dashboard. Staying local sends nothing, anywhere, ever.", + " Sessions include prompts, file contents and command output.", + " New to FailproofAI? Create a key at https://befailproof.ai/get-started/", ], // Cloud first, and therefore preselected: connecting is what most people // running this wizard came to do, and the local path stays one keystroke @@ -1272,14 +1422,26 @@ export async function runConfigureWizard(io: WizardIO = {}): Promise; + /** Age of the oldest parked batch, in ms. Undefined when none are parked. */ + oldestAgeMs?: number; +} + +const EMPTY: DeliveryHealth = { + total: 0, + rejected: 0, + credentialRejected: 0, + byStatus: {}, +}; + +/** + * Summarise `failed/` without touching it. + * + * A missing directory is the healthy case on a machine that has never had a + * batch refused, not an error — the collector creates it lazily. + */ +export function deliveryHealth(home?: string, now: number = Date.now()): DeliveryHealth { + const dir = failedDir(home); + if (!existsSync(dir)) return EMPTY; + + let entries: string[]; + try { + entries = readdirSync(dir); + } catch { + // Unreadable is reported as healthy rather than thrown: this runs on the + // hook path, and a status reader must never be the reason a tool call fails. + return EMPTY; + } + + const byStatus: Record = {}; + let total = 0; + let rejected = 0; + let credentialRejected = 0; + let oldestMtime: number | undefined; + + for (const name of entries) { + // `.tmp` files are half-written renames, not parked batches. + if (!name.includes(".jsonl")) continue; + total += 1; + + const parsed = parseParkedName(name); + if (parsed.clientStatus !== undefined) { + rejected += 1; + byStatus[parsed.clientStatus] = (byStatus[parsed.clientStatus] ?? 0) + 1; + if (CREDENTIAL_STATUSES.has(parsed.clientStatus)) credentialRejected += 1; + } + + try { + const mtime = statSync(join(dir, name)).mtimeMs; + if (oldestMtime === undefined || mtime < oldestMtime) oldestMtime = mtime; + } catch { + // Vanished mid-scan — the collector just drained or renamed it. + } + } + + return { + total, + rejected, + credentialRejected, + byStatus, + ...(oldestMtime !== undefined ? { oldestAgeMs: Math.max(0, now - oldestMtime) } : {}), + }; +} + +/** `2h 14m`, `3d`, `just now` — coarse on purpose; this is a "how stale" signal. */ +export function describeAge(ms: number): string { + const mins = Math.floor(ms / 60_000); + if (mins < 1) return "just now"; + if (mins < 60) return `${mins}m`; + const hours = Math.floor(mins / 60); + if (hours < 24) return `${hours}h ${mins % 60}m`; + return `${Math.floor(hours / 24)}d ${hours % 24}h`; +} + +/** + * The one-line verdict, or undefined when there is nothing to say. + * + * Deliberately silent unless a batch was DEFINITIVELY refused. Batches parked + * after exhausting server-error retries carry no client status and are picked + * up again by the retry pass, so reporting them would cry wolf over a blip the + * daemon is already handling. A credential the server refuses is the opposite: + * it never resolves on its own, and every session that passes without saying so + * is another session of data quietly not arriving. + */ +export function deliveryHealthLine(health: DeliveryHealth): string | undefined { + if (health.rejected === 0) return undefined; + + const batches = `${health.rejected} batch${health.rejected === 1 ? "" : "es"}`; + const age = health.oldestAgeMs !== undefined ? `, oldest ${describeAge(health.oldestAgeMs)}` : ""; + + if (health.credentialRejected > 0) { + const codes = Object.keys(health.byStatus) + .map(Number) + .filter((c) => CREDENTIAL_STATUSES.has(c)) + .sort(); + return ( + `Ingest REJECTED (${codes.join("/")}) — ${batches} parked${age}. ` + + `The key was accepted when it was connected and is not being accepted now; ` + + `nothing from this machine is reaching the dashboard. ` + + `Re-run \`failproofai config --connect\` with a working key.` + ); + } + + const codes = Object.keys(health.byStatus).map(Number).sort().join("/"); + return ( + `Ingest REJECTED (${codes}) — ${batches} parked${age}. ` + + `The server refused these and they are not retried; nothing from this ` + + `machine is reaching the dashboard.` + ); +} diff --git a/src/hooks/enforcement-capability.ts b/src/hooks/enforcement-capability.ts index 0799f981..e73cc45a 100644 --- a/src/hooks/enforcement-capability.ts +++ b/src/hooks/enforcement-capability.ts @@ -26,11 +26,19 @@ // ABSENT — NOT VERIFIED. The UI must stay silent, never assume "block". // // Only SOURCE / VENDOR_DOCS / LIVE_PROBE rows appear here. Every line carries -// its evidence. Versions probed: claude 2.1.220 · codex fe01054a · copilot -// 1.0.71 (some sites re-read in 1.0.68) · cursor-agent 2026.07.16-899851b · -// opencode 7565e035 (v1.18.9, re-checked at v1.14.33) · pi 0.80.10 (source -// 0.82.1) · hermes-agent 5771a6e · openclaw f8ed8ecf (v2026.7.2) · droid -// 0.175.1 · devin 3000.2.17 · agy 1.1.8 · goose 1.43.0. +// its evidence. Versions probed: claude 2.1.220 · codex fe01054a (PostToolUse +// re-probed LIVE at 0.147.0) · copilot 1.0.71, some sites re-read in 1.0.68 +// (PostToolUse re-read in the shipped 1.0.78 bundle) · cursor-agent +// 2026.07.16-899851b · opencode 7565e035 (v1.18.9, re-checked at v1.14.33) · +// pi 0.80.10 (source 0.82.1) · hermes-agent 5771a6e · openclaw f8ed8ecf +// (v2026.7.2) · droid 0.175.1 · devin 3000.2.17 · agy 1.1.8 · goose 1.43.0. +// +// A VERSION HERE IS PART OF THE CLAIM, NOT A FOOTNOTE. Re-probing codex for the +// PostToolUse row surfaced that its hook sources were restructured after +// fe01054a — output_parser.rs, hook_runtime.rs and tools/registry.rs, cited by +// name and line in several codex rows below, no longer exist at those paths in +// 0.147.0. Those rows are not known-wrong, they are unverified against any +// shipping codex; treat them as due for a re-probe rather than as evidence. import type { HookEventType, IntegrationType } from "./types"; export type EnforcementCapability = "block" | "observe"; @@ -87,7 +95,7 @@ export const ENFORCEMENT_CAPABILITY: Record< UserPromptSubmit: "block", // events/user_prompt_submit.rs:226 Some(2)+stderr -> should_stop; turn.rs:588 skips record_pending_input, turn.rs:237 returns. CAVEAT turn.rs:603 — a co-submitted accepted message lets the turn proceed Stop: "block", // events/stop.rs:303 Some(2) -> should_block + continuation_prompt; turn.rs:473-489 records it and `continue`s. CAVEAT turn.rs:490 a block with no prompt is warned + ignored SubagentStop: "block", // shared parse_completed (stop.rs:202) + same consumer turn.rs:473. CAVEAT hook_runtime.rs:345 — only ThreadSpawn subagents dispatch it; all other SubAgent sources never run the hook - PostToolUse: "observe", // codex CAN block here (registry.rs:678) but we emit hookSpecificOutput.additionalContext with no top-level `decision`, and output_parser.rs:215 needs decision==="block". Inert by our choice; tool already ran regardless + PostToolUse: "block", // FIXED: we now emit top-level {decision:"block",reason}. LIVE_PROBE codex 0.147.0 A/B (identical prompt+hook, only the shape differs): block shape -> `hook: PostToolUse Blocked` + codex_core::tools::router error=, and the reason REPLACES the tool result (the model never saw the real stdout); hookSpecificOutput -> `hook: PostToolUse Completed`, model read stdout verbatim. Result-replacement, not prevention — the tool already ran. NOTE the other codex rows below cite output_parser.rs / hook_runtime.rs / tools/registry.rs paths that DO NOT EXIST in 0.147.0 (hooks live at hooks/src/{engine,events}/…); they are unre-verified since fe01054a SessionStart: "observe", // events/session_start.rs:243-322 has NO Some(2) arm — our exit-2 deny is logged as HookRunStatus::Failed. CLI blocks only on exit 0 + {"continue":false} (session_start.rs:273 -> turn.rs:233) SubagentStart: "observe", // permanently unblockable: session_start.rs:273 guard excludes it, output schema has no `decision`, vendor doc says continue:false does not stop it PreCompact: "observe", // compact.rs:246-260 no Some(2) arm; latent block needs {"continue":false} (-> core/src/compact.rs:190 TurnAborted). Inert as we emit @@ -105,7 +113,7 @@ export const ENFORCEMENT_CAPABILITY: Record< SubagentStop: "block", // 1.0.71 app.js@1074101 subagentStop: decision==="block" && reason -> `continue` re-runs the subagent turn. CAVEAT skipped entirely for isSidekick subagents UserPromptSubmit: "block", // FIXED: we now emit {decision:"block",reason} at exit 0 (gate V$t @2547438, consumer @2823018). Was inert — we sent exit 2 + stderr, which copilot logs as a warning for EVERY event and never treats as a deny PermissionRequest: "block", // FIXED: we now emit the FLAT {behavior,message} copilot parses (normalizer CMn @179042 -> mapper h4t @2686538). Was inert — the Codex-shaped nested hookSpecificOutput.decision normalized to {} - PostToolUse: "observe", // OVERTURNED. Gate vK @173853 needs top-level decision==="block"; we emit hookSpecificOutput.additionalContext. Post-hoc anyway (result-rewrite only). Vendor docs: "Can block? No" + PostToolUse: "block", // FIXED: we now emit top-level {decision:"block",reason}. Re-read in the SHIPPED 1.0.78 app.js: BOTH postToolUse call sites gate on vK = t => t?.decision==="block" && typeof t.reason==="string" -> "Tool result blocked by {policy hook|hook}" / `Tool result blocked: ${reason}`. vK fails closed on a missing or non-string reason. Result-rewrite, not prevention — the tool already ran (vendor docs still say "Can block? No", which is true of the SIDE EFFECT, not of the result the model reads) SessionStart: "observe", // 1.0.71 app.js@2836633 reads ONLY additionalContext SessionEnd: "observe", // 1.0.71 app.js@2627263 `.then(()=>{})` — return explicitly thrown away PostToolUseFailure: "observe", // 1.0.68 app.js@2883424 and @3038111 destructure only additionalContext diff --git a/src/hooks/fp-config.ts b/src/hooks/fp-config.ts index b089bec5..78121448 100644 --- a/src/hooks/fp-config.ts +++ b/src/hooks/fp-config.ts @@ -212,7 +212,7 @@ export function writeVersionFile( // ── config.json ────────────────────────────────────────────────────────────── /** - * How this machine relates to Failproof Cloud. + * How this machine relates to FailproofAI Cloud. * * `oss` is not merely "no credentials configured" — it is a hard gate. In OSS * mode nothing constructs a cloud URL, polls for policy, or spools an event diff --git a/src/hooks/handler.ts b/src/hooks/handler.ts index c093afd5..81606f16 100644 --- a/src/hooks/handler.ts +++ b/src/hooks/handler.ts @@ -34,6 +34,7 @@ import { clearPolicies, registerPolicy, getPoliciesForEvent } from "./policy-reg import { loadAllCustomHooks } from "./custom-hooks-loader"; import type { CustomHook } from "./policy-types"; import { persistHookActivity } from "./hook-activity-store"; +import { deliveryHealth, deliveryHealthLine } from "./delivery-health"; import { trackHookEvent, flushHookTelemetry } from "./hook-telemetry"; import { resolveCwd } from "./resolve-cwd"; import { resolvePermissionMode } from "./resolve-permission-mode"; @@ -611,6 +612,31 @@ export async function handleHookEvent(eventType: string, cli: IntegrationType = const result = await evaluateHookEvent(eventType, cli, stdinRead.payload); + // Say it out loud, once a session, when the collector is holding batches the + // server definitively refused. + // + // Everything `--status` prints about the connection comes from the credential + // file, which records what was true at `--connect` time and is never + // revisited — so a revoked or expired key, or an org that was disabled, + // leaves that file correct while nothing arrives. `--status` also only speaks + // when someone runs it, and the whole failure mode here is that nobody knows + // there is anything to ask about. SessionStart is the one place failproofai + // is already invoked, on every CLI, exactly once per session, with a person + // watching — so this costs one directory read per session and needs no flag, + // no daemon channel and no user action. + // + // Never allowed to affect the outcome: the read swallows its own errors, the + // verdict goes to stderr (SessionStart is `observe` everywhere — a stderr + // write there cannot block a session), and `result.exitCode` is untouched. + try { + if (canonicalizeEventType(eventType, cli) === "SessionStart") { + const rejection = deliveryHealthLine(deliveryHealth()); + if (rejection) hookLogWarn(rejection); + } + } catch { + // A status notice must never be the reason a hook misbehaves. + } + if (result.stdout) { process.stdout.write(result.stdout); } diff --git a/src/hooks/policy-evaluator.ts b/src/hooks/policy-evaluator.ts index ce20e195..cce0235b 100644 --- a/src/hooks/policy-evaluator.ts +++ b/src/hooks/policy-evaluator.ts @@ -497,6 +497,43 @@ export async function evaluatePolicies( } if (eventType === "PostToolUse") { + // codex and copilot both gate PostToolUse on a TOP-LEVEL + // {decision:"block", reason:}. Neither reads the + // hookSpecificOutput shape below, so every PostToolUse deny on them was + // emitted, logged, counted as enforcement — and dropped on the floor. + // + // codex 0.147.0, live A/B probe (same prompt, same hook, only the shape + // differs): with {decision:"block"} the run prints `hook: PostToolUse + // Blocked`, routes the reason through codex_core::tools::router, and the + // reason REPLACES the tool result — the model never sees the real + // output. With hookSpecificOutput it prints `hook: PostToolUse + // Completed` and the model reads the output verbatim. NOTE the codex + // rows elsewhere cite output_parser.rs / hook_runtime.rs / tools + // registry.rs line numbers that no longer exist in 0.147.0 (its hook + // sources are hooks/src/{engine,events}/…); this row is re-grounded on + // the live probe rather than those paths. + // + // copilot 1.0.78: BOTH postToolUse call sites in the shipped + // @github/copilot-linux-x64 app.js gate on + // vK = t => t?.decision === "block" && typeof t.reason === "string" + // -> "Tool result blocked". vK fails closed on a missing or non-string + // reason, so the reason must always be a non-empty string — do not + // "simplify" this to {decision:"block"} alone. + // + // The tool has already RUN in both cases. This replaces the result the + // model reads; it does not undo the side effect. That is the only + // semantic available at PostToolUse, and it is precisely what an + // output-scrubbing policy needs to keep a secret out of the context. + if (session?.cli === "codex" || session?.cli === "copilot") { + return { + exitCode: 0, + stdout: JSON.stringify({ decision: "block", reason: blockedMessage }), + stderr: "", + policyName: policy.name, + reason, + decision: "deny", + }; + } const response = { hookSpecificOutput: { hookEventName: eventType, diff --git a/src/hooks/policy-presets.ts b/src/hooks/policy-presets.ts index 5d515be5..6a6a86ab 100644 --- a/src/hooks/policy-presets.ts +++ b/src/hooks/policy-presets.ts @@ -69,3 +69,67 @@ export function resolvePreset(id: string): string[] { export function resolveEverything(): string[] { return BUILTIN_POLICIES.filter((p) => !p.beta).map((p) => p.name); } + +/** + * What "Recommended" turns on — the set someone gets for pressing Enter once. + * + * WRITTEN OUT, not derived. `defaultEnabled` is the seed for the *policy list* + * prompt and answers a narrower question ("tick this by default in a list of + * 40"); this answers "what should guard a machine whose owner did not want to + * choose". They overlap heavily and are not the same promise, and deriving one + * from the other would silently change this set every time somebody flipped a + * flag on an unrelated policy. `recommendedCoversEveryDefault()` keeps them + * from drifting APART without anyone noticing. + * + * Nothing here needs configuring to be useful and nothing here has a common + * false positive — that is the bar for being in this list, because the person + * who chose Recommended is the person least equipped to debug a bad deny. + * + * Deliberately EXCLUDED, and why: + * • Workflow (`require-*-before-stop`) — refuses to let the agent finish + * until CI is green. A defensible choice, never a default one, and it does + * not fire at all on hermes or goose (see enforcement-capability.ts). + * • Infra Commands — blocking kubectl/terraform/aws breaks the day job of + * anyone doing infra. That is what the "Cloud & infra" bundle is for. + * • block-read-outside-cwd — agents legitimately read outside the repo. + * • block-work-on-main — plenty of people work on main on purpose. + * • the warn-* family — non-blocking and cheap, but ten warnings is noise, + * and a warning nobody reads is worse than one that was never shown. + */ +export const RECOMMENDED_POLICIES: readonly string[] = [ + // Secrets never reach the model, and never reach disk. + "sanitize-jwt", + "sanitize-api-keys", + "sanitize-connection-strings", + "sanitize-private-key-content", + "sanitize-bearer-tokens", + "protect-env-vars", + "block-env-files", + "block-secrets-write", + // The agent cannot disable its own guardrails. + "block-self-pause", + "block-failproofai-commands", + // Commands that are unrecoverable when they are wrong. + "block-sudo", + "block-curl-pipe-sh", + "block-rm-rf", + // Git history stays recoverable. `--force-with-lease` is still allowed. + "block-push-master", + "block-force-push", +]; + +/** + * Every `defaultEnabled` policy that Recommended does NOT include. + * + * Empty today, and a test asserts it stays that way. The point is the day + * someone adds a new default-on builtin: Recommended is a separate list and + * would silently not include it, so a machine set up by pressing Enter would + * be guarded LESS than one set up through the policy list — the kind of gap + * that is invisible until somebody is standing in it. + */ +export function defaultsMissingFromRecommended(): string[] { + const recommended = new Set(RECOMMENDED_POLICIES); + return BUILTIN_POLICIES.filter((p) => p.defaultEnabled && !p.beta && !recommended.has(p.name)).map( + (p) => p.name, + ); +}