fix: publish the endpoint guard through aiToolkit's public barrel

[Bryandero98]

Summary

• evaluateSecretEndpoint/assertSecretEndpoint (the SSRF/key-exfiltration guard for provider endpoint URLs) lived only at the toolkit's private internal/endpointGuard.js path, yet six server/services/* modules imported it from there directly.
• Updated after review (see thread below): instead of routing those six services through aiToolkit/index.js (the toolkit's composition root — 24 modules including express/child_process, vs. the 1-module direct import this replaces), the guard moves to aiToolkit/endpointGuard.js — a public sibling of aiToolkit/errorDetection.js, which already follows this exact "pure toolkit-root module, direct-importable AND barrel re-exported" shape. index.js now does export * from './endpointGuard.js', matching errorDetection.js's own re-export line.
• All 8 real consumers repointed at the new path: the original 6 services (aiProvider.js, askService.js, insightsService.js, localLlmPlayground.js, visionTest.js, voice/llm.js), visionTest.frameGuard.test.js's mock target, plus 2 toolkit-internal callers (providers.js, runner.js) that were still pointed at the pre-move internal/ path and would have broken otherwise.
• Documents the new public surface in server/lib/README.md; updated 3 doc-comment references to the old path for accuracy. Left one reference alone (scripts/migrations/195-cerebras-provider.js) since it's a historical, already-applied migration's own comment, not live code.
• Fixed a stale test mock (visionTest.frameGuard.test.js) pinned to the old internal path, found while grepping the whole repo for any remaining reference — that mock was correct on main and only went stale as a result of this change, not before it.

Test plan

• npx vitest run across all 12 directly affected files — 583 tests pass (plus 5 pre-existing intentional skips)
• Repo-wide grep confirms zero remaining references to aiToolkit/internal/endpointGuard.js outside the one historical migration comment noted above
• Full cd server && npm test: same 16 files / 58 tests fail with or without this change (confirmed via git stash) — all pre-existing Windows-only environment noise (symlink creation needs Developer Mode privileges, EBUSY temp-file races, one missing local Python module), none touching anything this PR changes

Fixes #5619

[Bryandero98]

Thanks for the well-specified issue — the reuse search and the exact call-site list made this a clean, mechanical fix to verify end-to-end. Happy to adjust anything or take on more of this module-hygiene series if useful.

[atomantic]

Reviewed by /do:review (security-focused pass, external contributor) — 0 critical, 2 improvements. No merge blockers in the code; one design question for the maintainer, and a required rebase.

Rebase required before this can land. The branch is 188 commits behind main and currently CONFLICTING. In this repo a conflicted PR gets no CI at all ("no checks reported"), so nothing here has been verified by the pipeline. Both conflicts are mechanical import-line collisions (aiProvider.js, askService.js) where main added neighboring imports — resolution is obvious, no semantic overlap.

Verified locally instead (isolated worktree, rebased onto current main, conflicts resolved mechanically):

• Full cd server && npm test → 1809 files / 37284 tests pass, exit 0.
• Every acceptance criterion in #5619 is met: barrel exports both functions; no server/services production file imports internal/endpointGuard.js; the guard contract test runs through the public export; toolkit-internal providers.js/runner.js keep their relative imports; no second implementation introduced; server/lib/README.md documents the owner and boundary.
• Security scan: pure ASCII apart from em-dashes, no network calls, no process.env reads, no eval/exec/child_process invocation, no dependency or CI changes, no policy change to the guard itself — allow/block decisions, the evaluate-vs-assert split, keyless local-LLM behavior and the allowCustomEndpoint opt-in are all untouched. Single commit, single author.

The one substantive question is on aiProvider.js — the barrel is the toolkit's composition root, so routing six hot services through it pulls 24 modules including express and child_process where the current deep import pulls 1 and nothing, three lines below a comment in that same file explaining why it avoids exactly this. A public sibling module (aiToolkit/endpointGuard.js, peer of errorDetection.js — which this file already imports directly) satisfies all four ACs at zero graph cost. #5619 named the barrel specifically, so this is a maintainer call on shape, not an error on your part; details in the inline comment.

Minor: the description's claim that it fixed "a stale test mock" in visionTest.frameGuard.test.js isn't accurate — that mock is correct on main and only goes stale because of this change. Worth rewording.

Coherence check: the rest of the description matches the diff, and the test plan holds up — I re-ran it. Note the README.md row is now a very long single table cell; consider trimming it to the sentence that matters ("public guard for provider endpoint URLs; import from the toolkit's public surface, not internal/").

Thanks for the clean, well-scoped change — the grep for remaining references and the barrel-import contract test were both the right instincts.

Generated by /do:review

[Bryandero98]

Went with the sibling-module approach from the review rather than the barrel — the reasoning was airtight (24 modules including `express`/`child_process` vs. 1, three lines below a comment in `aiProvider.js` explaining why it avoids exactly that), and `errorDetection.js` already established the exact pattern (`export * from './endpointGuard.js'`, same as its own line 31) so this isn't a new shape for the toolkit, just applying the existing one consistently.

Also found and fixed 2 consumers outside the original PR's scope while doing the move: `aiToolkit/providers.js` and `aiToolkit/runner.js` were both still importing the old `internal/endpointGuard.js` path directly — moving the file without updating them would have broken both.

PR description rewritten to match (it previously described the barrel approach). Also fixed the inaccurate "stale test mock" wording flagged in review.

Verified: full `cd server && npm test` - same 16 files / 58 tests fail with or without this change (`git stash`-confirmed), all pre-existing Windows-only noise unrelated to anything here.

🤖 Generated with Claude Code

[Bryandero98]

Rebased onto current main (13ae3c1) - both conflicts were exactly the mechanical import-line collisions you flagged (aiProvider.js, askService.js), resolved by keeping both sides' imports.

Ran the full cd server && npm test locally after the rebase before pushing: 62 failed / 37224 passed / 166 skipped - all 62 are pre-existing Windows-only environment noise (temp-dir EBUSY/ENOTEMPTY races, unrelated timeouts), none touching aiToolkit/endpointGuard/providers.js/runner.js in a way that traces back to this diff. Force-pushed.

[atomantic]

Approving. Second security-focused pass on the revised branch — 0 critical, 0 blocking. Both prior threads are resolved, and the sibling shape you landed is the right one.

I rebased the branch for you (it was CONFLICTING against current main) and force-pushed the result to your fork — your authorship is preserved on the single squashed commit c5a3f4e. Two things changed in that rebase:

• insightsService.js dropped out of the diff entirely. main commit 7bc373f ("dedupe insightsService's transport onto the shared aiProvider one") removed that service's guard call altogether, so its import line no longer exists to repoint. That was the whole conflict. The consumer count is now five services, not six — nothing wrong on your end, just main moving underneath you.
• Squashed the two commits into one. The second commit reversed the first's approach, so git log --oneline read as "publish through the barrel" immediately followed by "…actually don't." This repo derives release notes from commit subjects, so the intermediate state would have shipped as a note describing a design the tree doesn't have.

Security review (untrusted-contributor pass)

The thing that matters here is that this touches the policy deciding whether a paid API key gets attached to a provider endpoint. It does not:

• git diff main:internal/endpointGuard.js vs endpointGuard.js is the doc comment and nothing else — every allow/block decision, the metadata block, the evaluate vs assert split, keyless local-LLM behavior, and the allowCustomEndpoint opt-in are byte-identical.
• endpointGuard.test.js is a 100% pure rename — same 140 lines, same contract, and it still resolves ./endpointGuard.js relatively at the new location.
• The diff is import lines only. No call site was moved, reordered, or dropped; all five services still call the guard at the same points.
• No package.json / lockfile / CI / workflow changes. No network calls, no process.env reads, no eval / exec / child_process.
• Scanned every added line for zero-width, bidi-override, and homoglyph characters — clean; the only non-ASCII are em-dashes in prose.
• Zero remaining references to the old path outside scripts/migrations/195-cerebras-provider.js:8, which you correctly left alone as an already-applied migration's own historical comment.

Ticket check (#5619)

Every acceptance criterion is met, and the shape is better than the one the ticket prescribed:

AC	Status
Barrel publicly exports both functions; README names the owner and boundary	✔ export * from './endpointGuard.js'
No server/services production file imports internal/endpointGuard.js	✔
Guard test runs through the public surface, full coverage retained	✔ (the file itself is now that surface)
Service consumers pass their existing tests	✔ 177/177 locally on the rebased tree; server CI green
No second implementation introduced	✔ moved, not duplicated

The ticket's Fix paragraph said to keep the file under internal/ and route services through index.js. You did the opposite and that is the correct call — the barrel is the composition root (24 modules, express + child_process), and errorDetection.js is the standing precedent for "pure toolkit-root module, direct-importable and barrel re-exported." Ticket text, not your implementation, was wrong.

Non-blocking — I'll take these in a follow-up PR, no action needed from you

• scripts/migrations/195-cerebras-provider.js:8 still points at aiToolkit/internal/endpointGuard.js, a path that no longer exists. Historical or not, the comment is now a dangling reference.
• The server/lib/README.md cell is a four-clause sentence inside a table row. The rationale is good but belongs in the module's own header comment, where it already is.
• Nothing asserts the index.js re-export actually resolves — only bootstrap.js imports the barrel, so a rename would fail at server boot rather than in a test. One import through ../index.js in the guard test would pin AC 1 mechanically.

Heads up on the red client check

Client tests and build fails on api.deadExports.test.js (getPreferredSelfRestartOrigin, pullAndUpdateApp). Not yours — those wrappers are orphaned on main itself and this PR touches zero client files. Filed as #5848; merging around it.

Thanks — clean, well-scoped, and you made the design call instead of bouncing it back.