fix(mode): select the backend explicitly and refuse non-loopback egress under test

[andrei-hasna]

What

HASNA_KNOWLEDGE_API_URL + HASNA_KNOWLEDGE_API_KEY selected the cloud backend by mere presence. Both are exported in a login shell on the machine this was found on, and the tmux server carries them, so every pane inherited them: bun test reported 99 failures instead of 1 — 64 of them the identical "cloud API flip is active" refusal — while the rest of the suite operated against the live store believing it was isolated. The symptom named neither the cause nor the store, so each agent working this repo rediscovered it independently and prefixed runs with env -u.

Third confirmed instance of one defect shape (todos f22a263f); the mementos instance is closed (7c9ea749, PR #17), and emails is being handled separately by other agents. This PR is the knowledge half only.

Three changes

1. Request-boundary guard — src/net-guard.ts (the primary control). While NODE_ENV=test, an outbound request whose target is not loopback is refused before a socket is opened. Refusals never name the target host, matching the existing repo precedent of refusing cloud config "without leaking the URL".

Environment clearing is not the primary control and could not be. The mementos fix measured all three escape routes: a test file assigning the selectors at module scope runs after any one-shot clear (green run, writes over HTTP, rc=0); bun test runs many files in one process with one preload, so a single file's leak reaches every later file; and a bunfig.toml preload resolves from the cwd and is simply absent when the suite is invoked from a subdirectory. At the egress it no longer matters which layer decided to make the request or when the variable was set. This repo has no bunfig.toml at all, so a preload was never even available here.

Loopback stays allowed on purpose — tests that stand up a Bun.serve on 127.0.0.1 and drive the real transport are the good case, and refusing them would push the suite toward stubbing the transport, which is how an egress bug hides. There is no opt-out: every in-repo script that legitimately talks to a real endpoint (scripts/smoke-*.mjs) runs outside bun test.

2. Explicit mode selection — src/knowledge-mode.ts. Modelled on emails' resolveEmailsModeSelection: the first mode key carrying a value wins and returns, without reading the pointer keys, so KNOWLEDGE_MODE=local is authoritative on a machine that exports a URL and a key. With no mode key the answer is local.

Two layers were inferring cloud from presence, and both had to be closed:

• src/cloud-store.ts withInferredCloudMode() — deleted.
• resolveClientTransport in @hasna/contracts (dist/client/storage.js:6206-6209, else if (urlHit && keyHit) { mode = "cloud" }) — still infers independently. Deleting only the first would have changed nothing. Callers now hand it a mode-pinned env via pinnedTransportEnv(), in both directions, so its inference cannot fire and the two layers cannot disagree about which store is authoritative.

Env key lists come from clientTransportEnvKeys() in @hasna/contracts rather than being restated locally (the old code restated them with a comment admitting it was a mirror) — a key we ignored could otherwise still flip the transport underneath us. Mode values normalize through the vendored storage-kit normalizeStorageMode, so self_hosted keeps working as a deprecated spelling of cloud.

3. knowledge mode — a side-effect-free reporter. Prints the resolved backend, the env var that selected it, pointer vars present-but-ignored, and whether the guard is armed. Env var names only, never values. It reads the environment alone: no store opened, no config read, no request, so it answers on a machine with no config and no network.

Specifically checked for the trap the mementos equivalent hit — there, the mode command created a 720 KB database through a global preAction hook calling getDatabase() before every command. knowledge has no preAction hook, but run() did construct the service before command dispatch, so the handler is placed above that, ahead of every store, config and db touch.

Breaking, and who has to act

A fleet flip that writes only the two pointer vars now leaves the CLI local. It must also write HASNA_KNOWLEDGE_STORAGE_MODE=cloud. The flip writer is not in this repo (it lives in @hasna/machines/fleet tooling), so this needs a coordinated update before any machine is relied on to be in cloud mode. tests/cloud-store.test.ts's "fleet-flip writes no STORAGE_MODE" case is inverted in this PR, deliberately and with the reasoning recorded at the test.

Evidence

All test numbers below are with the ambient selector variables left exactly as they are — no env -u — because that is the condition that was broken.

	before	after
bun test	99 fail	306 pass / 2 skip / 1 fail across 309 tests in 42 files

The single remaining red is the pre-existing context pack and proposal context commands return bounded agent JSON (tests/cli.test.ts:3044), which also fails on main at 95c853d. Baseline on main with the vars neutralised: 276 pass / 2 skip / 1 fail across 279 tests in 40 files. No timeout-shaped failure names appeared, so the b87fd095 hardcoded-timeout artefact did not fire at current load. (FORCE_COLOR=3 is set here: grep -c '(fail)' returned 0 on runs that really had failures, while ✗ returned the true count — counts above come from the ✗ marker and the summary line.)

Socket count, strace -f -e trace=socket,connect, against the real configured endpoint under explicit cloud mode + NODE_ENV=test:

connects to :443           = 0
non-loopback INET connects = 0
AF_INET/AF_INET6 sockets   = 0
total connect() syscalls   = 0

Positive control — the identical command with the guard disarmed, pointed at a public harmless endpoint, so the measurement is shown able to detect what it claims: 4 connects to :443, 9 non-loopback INET connects, 7 INET sockets, and a real HTTP 404 came back. Without that control, "0 connects" would only have proven the strace filter was wrong.

The actual test-suite condition (NODE_ENV=test, pointers present, no mode var, nothing else changed): 0 connects, 0 sockets, and list returned rows from the on-box store.

Reporter side effects: six knowledge mode invocations in an isolated HOME and cwd produced byte-identical file trees before and after (excluding bun's own transpile cache, which is the runtime's, not this CLI's).

Tests proven red against the planted old behaviour, run after committing (so git checkout -- <file> restores the fix rather than deleting it):

• Plant A — presence-inference restored in the resolver: 6 red, the three REGRESSION: cases plus resolveItemStore returns the LOCAL transport under a pointer-only env, the isKnowledgeApiMode/resolveKnowledgeCloudStore agreement case, and the report's pointer-name case.
• Plant B — isNetworkGuardActive forced to false: 7 red, including both transport-boundary egress cases.
• Both reverted, tree clean at the same HEAD, 41/41 green.

bun run verify:generated → rc=0, "6 generated bundles rebuild byte-identically and carry no stale generated code". Rebuilt with bun 1.3.14, matching the ci.yml pin, since the byte gate is bundler-version-sensitive. Note that CI cannot be relied on to catch a bundle mistake here: Verify generated artifacts is skipped in every job that defines it (HC-00201) because the preceding Run tests step fails and it has no if: always().

Also fixed, same family

tests/cli.test.ts handed the ambient pointer vars to every spawned CLI child. auth whoami reports authenticated: true from the mere presence of an API key (src/auth.ts), so the hosted-auth contract test was measuring the developer's shell instead of the temp auth dir it had just created — a second red under ambient env, independent of the mode fix (verified: it passes on this branch with env -u, fails without). Spawned children now go through one childEnv() helper that strips those keys, with the strip list imported from the contract rather than restated. Labelled in-code as defence in depth, explicitly not the control.

Not done / assumed

• Not audited: the full @hasna CLI sweep that f22a263f also asks for. Out of scope here; emails untouched by request.
• Not changed: @hasna/contracts' own presence-inference. It is neutralised for knowledge by pinning, but every other consumer of that package still has it. Worth its own task.
• Not added: an MCP tool mirroring knowledge mode. The reporter is exported from the SDK (knowledgeModeReport) so the MCP surface can pick it up, but no tool was added.
• 0.0.0.0 and [::] are deliberately not treated as loopback — they are bind-any addresses, and accepting a wildcard as safe is how a guard gets talked past.

Refs: todos f22a263f. Not self-merged — ready for an adversarial review pass.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[andrei-hasna]

Second commit 691cb7c — self-review correction, recorded here rather than by editing the PR body above.

The guard's header claimed it covered "every knowledge-owned outbound HTTP request" while src/source-ingest.ts readWebText called global fetch directly, so under NODE_ENV=test a web:///https:// source ref would still have reached the network. Nothing in the suite ingests a web ref, which is exactly why that was easy to leave open: the cloud transport was the visible hole and this one was not.

• readWebText now uses guardedFetch.
• New test: a non-loopback web source-ref ingest is refused, and the refusal does not name the host. Plant-verified — reverting the one guardedFetch call turns that test red (1 red, and only that one), then green again on revert.
• The header now states the scope instead of overclaiming. Covered: the cloud item transport and web source ingestion. Not covered: third-party SDK transports with their own HTTP stacks (@aws-sdk/client-s3, the ai provider clients), which are credential-gated and driven with --fake in tests. A guard people believe covers more than it does is worse than a narrow one.

Updated numbers, ambient selectors still left exactly as they are: bun test 307 pass / 2 skip / 1 fail across 310 tests in 42 files (the body says 306/309 — that was before this commit). The single red is unchanged and pre-existing: context pack and proposal context commands return bounded agent JSON at tests/cli.test.ts:3044, also red on main at 95c853d. bun run verify:generated rc=0 again, 6 bundles byte-identical on bun 1.3.14.

[andrei-hasna]

Adversarial review — APPROVE WITH FIXES (fixes pushed as 43a61dc)

Independent review in my own clone at $HOME/.hasna/repos/worktrees/knowledge/adv-pr43-mode-guard, bun 1.3.14 (== the ci.yml pin), aarch64 Linux, bun install --frozen-lockfile = 156 packages. Every measurement below was taken with the ambient selector variables left exactly as they are — no env -u, no scrubbing — because a verification run under a clean environment would prove nothing about the condition this PR exists to fix. Confirmed present in my shell before measuring: HASNA_KNOWLEDGE_API_URL, KNOWLEDGE_API_URL, HASNA_KNOWLEDGE_API_KEY; no mode var.

The change is sound and the central claims reproduce. I found one real hole in the guard, fixed it, and corrected two stale artefacts. Merging after CI.

---

1. Headline measurement, reproduced independently

bun test, ambient env, at 691cb7c: 307 pass / 2 skip / 1 fail across 310 tests in 42 files, 140.02s. Sole red: knowledge cli > context pack and proposal context commands return bounded agent JSON (tests/cli.test.ts:3070 — the case cited as :3044, moved by the childEnv insertion). Zero occurrences of the cloud API flip is active signature. Matches the claim exactly.

FORCE_COLOR trap confirmed live in my own run, as briefed: on a run that really failed, grep -c '(fail)' returned 0 while grep -c '✗' returned the true 1. Every count here comes from ✗ plus the ANSI-stripped summary. (The inverse holds in CI, where colour is off and (fail) is the only form present — so both greps are genuinely required depending on where the log came from.)

2. Two inference layers — confirmed, and the pin is genuinely authoritative

Both layers verified in source, not inferred:

• @hasna/contracts dist/client/storage.js:6206-6209 — else if (urlHit && keyHit) { mode = "cloud" }, reached only when modeHit is null.
• modeHit = firstEnv2(env, keys.modeKeys) at :6191, and clientTransportEnvKeys at :6154-6166 orders modeKeys canonical-first — the same list knowledge-mode.ts imports. pinnedTransportEnv stamps KNOWLEDGE_MODE_ENV_KEYS[0], i.e. the highest-priority key in the very order contracts scans, so the presence branch is unreachable once pinned. Not merely usually-shadowed: structurally unreachable.
• resolveStorageClient(name, env, overrides) at :6542 does accept the third argument, and createClientTransport forwards fetchImpl (used at :6313/:6357) and retry. So the guard really is installed on the transport rather than silently dropped.

One correction to how it is described, not to what it does: the claim is a mode-pinned env "in both directions". The cloud direction is pinned. The local direction is stronger than pinning — resolveKnowledgeCloudStore returns null before contracts is called at all, so nothing is handed to it. pinnedTransportEnv(env, 'local') is exercised only in tests/knowledge-mode.test.ts:180. The outcome is right; "in both directions" describes the function's capability rather than the live call path.

3. Egress guard — I attacked its arming, and it holds

Construction-time versus request-time is the load-bearing choice, so I tried to defeat it directly: construct the cloud client with NODE_ENV=production, then arm NODE_ENV=test after construction and issue the request. Refused — KnowledgeNetworkGuardError. The reverse (construct armed, disarm before the request) is not refused, which is the documented per-request semantics and correct.

Minor asymmetry worth knowing: retry: false in transportOverrides is decided at construction, while arming is decided per request. A client constructed while disarmed and then armed spends ~840 ms in backoff before surfacing the refusal (measured). Latency only, never a leak.

strace, re-run rather than taken on trust — real configured endpoint, explicit cloud mode, NODE_ENV=test, read-only list (never a write):

condition	connect() total	connects to :443	AF_INET/INET6 sockets	non-loopback INET connects	outcome
armed, real endpoint, explicit cloud	0	0	0	0	KnowledgeNetworkGuardError
positive control: same command disarmed, public endpoint	13	4	7	9	real HasnaHttpError
actual suite condition: pointers set, no mode var, NODE_ENV=test	0	0	0	0	resolved local, no transport built

The control reproduces the reported 4/9/7 to the digit, so the zeros are the guard working and not my filter being wrong.

4. FINDING (fixed in 43a61dc): the loopback allowance was a springboard

The brief asked whether the loopback allowance can be abused to reach a non-loopback address. It could. guardedFetch asserted only the first target and then handed the request to fetch, which follows a 3xx internally — and an internal hop never comes back through a fetchImpl. A hermetic 127.0.0.1 server answering 302 with an off-box Location therefore left the machine through a request the guard had already approved.

Measured before the fix, strace on a loopback server redirecting to a public host: 4 connects to :443, 9 non-loopback INET connects, HTTP 200 returned from off-box. The module header's claim that a non-loopback target "is refused before a socket is opened" was not true for redirect targets.

Fixed: while armed, the guard follows redirects itself and asserts every hop, refusing an off-box hop with the same host-withholding error (bounded to 5 hops, with fetch's own 301/302/303 method downgrade). Unarmed it is plain fetch with plain fetch redirect handling, so the production path is untouched. Two tests added; the off-box-hop one is red without the fix (verified red first), and the same strace probe now makes 0 connects.

Also checked, and not a defect: *.localhost is allowed by name rather than by address, so a resolver mapping x.localhost to a routable address would slip through. On this machine getent hosts evil.localhost returns ::1. Left as-is — address-based checking would mean the guard doing its own DNS.

5. FINDING (fixed): the message an operator actually hits still described the deleted rule

src/knowledge-db.ts assertLocalCatalogMode still said the refusal was because "the cloud API flip is active (HASNA_KNOWLEDGE_API_URL + HASNA_KNOWLEDGE_API_KEY set)" and instructed "Unset the API env". After this PR that is wrong in both halves — the pointers are not why the process is in cloud mode, and unsetting them does not restore local; with the mode var still set it produces a misconfiguration error instead. This is the loudest user-facing message in the repo and it was left telling operators to edit the wrong variable, which is precisely the confusion the PR set out to end. The README was updated thoroughly; this message and the item-Store comment at src/cli.ts:936 were missed. Both corrected, with the mode key sourced from KNOWLEDGE_MODE_ENV_KEYS rather than restated. No test asserted on the old text.

6. FINDING (fixed): stale count in the CHANGELOG

The CHANGELOG claimed 306 / 2 / 1 across 309 — the a4c1460 measurement, not updated when 691cb7c added a test. Corrected to the count at this tip and separated from the CI figure it is compared against.

7. Plants re-run — both confirm, one is stronger than reported

Both planted after committing, both reverted, tree clean at the same HEAD each time.

• Plant A (presence-inference restored in the resolver): 78 red. Exactly 6 are the mode contract — the two REGRESSION: cases, the inverted stays local when ONLY API url+key are set, resolveItemStore returns the LOCAL transport under a pointer-only env, isKnowledgeApiMode and resolveKnowledgeCloudStore agree, and the mode report. That 6 matches the reported figure. The other 72 are the ambient-env artefact flood the PR exists to remove — 66 carrying the opening the local knowledge.db refusal, i.e. the same signature as the original 64. Re-planting the defect reproduces the flood; that is the strongest single piece of evidence in this PR and it was under-reported.
• Plant B (isNetworkGuardActive forced false): 10 red, of which 9 are plant-caused (the tenth is the pre-existing context-pack case) — reported as 7. Both transport-boundary egress cases are among them, plus the source-ingest case and, usefully, the this very suite runs with the guard armed tripwire.

Two count corrections to the report: there are two REGRESSION:-labelled cases, not three (tests/knowledge-mode.test.ts:59 and :74); and Plant B is 9, not 7. Both errors are in the conservative direction.

8. CI — the traps are real, and the report understated the coverage

gh pr checks shows 7 failed/pending; the job conclusions at 691cb7c are 2 failure + 5 cancelled (fail-fast). Confirmed.

Reading the run log rather than the summary: four jobs reached and completed the test step, not three — test (ubuntu, bun), test (macos, bun), test-matrix (ubuntu, bun), test-matrix (macos, bun) — all four 307 pass / 1 fail / 310 tests across 42 files, with the same sole failure. test (ubuntu, bun) finished its suite before being cancelled at a later step, which is why it was missed.

Verify generated artifacts appears 0 times in the full run log, not just the failed log — HC-00201 confirmed. Run locally instead: bun run verify:generated → rc=0, 6 bundles rebuild byte-identically, both at 691cb7c and again at 43a61dc after rebuilding. Still aarch64, so the x64 caveat on the byte gate stands unchanged. Windows remains cancelled, therefore unverified — and so do both node jobs.

9. auth whoami — a real defect, and it belongs in its own task

Confirmed at src/auth.ts:101-118: authenticated: Boolean(key.apiKey). Authentication is reported from the presence of a key, with no session verified — the same presence-means-truth shape as the defect this PR fixes, one layer up. The finding is correct and the childEnv() remedy here is the right scope: it stops the suite measuring the developer's shell, which is a test-isolation fix.

Ruling: out of scope for this PR, needs its own task. Making whoami truthful means either verifying the session (a network call, which the guard would then refuse under test — a design question in itself) or renaming the field and deprecating it. api_key_present already carries the honest signal, so authenticated is a misleading duplicate rather than a missing capability. That is a contract change to a shipped command with its own consumers, and bundling it here would put an unrelated behavioural change behind a mode-selection fix.

10. Two deliberate omissions — both correct calls

• Not reaching the live store, and not reproducing the defect the obvious way. Reproducing through the pure resolver is sound and I re-ran it: at 691cb7c under ambient pointers, resolveKnowledgeCloudStore → null, isKnowledgeApiMode() → false, resolveItemStore → LocalItemStore, with and without NODE_ENV=test. Plant A then re-creates the failure flood from the same environment without any live traffic. Refusing to re-run the pre-fix suite under ambient env was right: not all 99 failures were local throws, and the count was already recorded elsewhere. The .invalid reserved TLD for every fake pointer is the correct choice.
• The @hasna CLI presence-selection audit is not done and the task stays open. Agreed — that is a separate sweep with its own control requirements, not a condition of landing this fix.

11. The breaking change stands — I did not soften it

No compatibility fallback added. A fleet flip writing only the two pointer vars now leaves the CLI local, and it must also write HASNA_KNOWLEDGE_STORAGE_MODE=cloud; that writer lives in @hasna/machines, not here. The [BREAKING] notice is real — I read #announcements 603451 back by id rather than through a digest. conversations blockers reports no blocking message.

Merging is safe ahead of the @hasna/machines change because merging to main flips no machine — the behaviour only reaches a box when the package is published and installed. This machine is a live example: it carries the pointers with no mode var, so its installed CLI is on cloud today and would become local on upgrade. Publish is the gate, not merge. I have not published and would not without the machines-side change landing first.

Unverified, stated plainly

• Windows and both node CI jobs: cancelled by fail-fast, so unverified. No path handling is involved in the guard or the resolver, and the node jobs run bun test regardless, so the residual risk is low — but it is not measured.
• The byte gate on x64: still only checked on aarch64.
• The original "99 failures, 64 sharing one error": not re-measured, by design — it would require running the pre-fix suite against the live store. Plant A's 78 red / 66 same-signature is the closest independent analogue.
• The @hasna/machines flip writer: not inspected. The local open-machines checkout on this box has no working tree.