Audit fixes: revive the API, close the token leak, make self-hosting the default - #19
Merged
Conversation
tracedGenerate (apps/api/src/gemini.ts:41) called itself at line 57 instead of ai.models.generateContent(params). Every LLM call unwound into RangeError: Maximum call stack size exceeded before any network I/O, so all ten exported functions were dead — /score, /coach, /diff and /improve with them. The recursion was type-valid, so tsc --noEmit and CI stayed green the whole time. apps/api had no test script and zero tests, which is exactly why this shipped. Add gemini.test.ts: seven tests that stub globalThis.fetch and assert a request actually reaches the transport, which is precisely what the recursion prevented. Verified as a real regression guard — reintroducing the self-call turns the suite red with RangeError rather than merely failing an assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…en; add LICENSE
/coach used to answer a scoring failure with { proceed: true, text: '',
overall: 0 }. The MCP coach tool renders an empty text on a proceed=true
score-mode response as "(coach overall: 0/10 - no coaching needed)",
byte-identical to what it prints for a flawless prompt. So whenever
Gemini was down or returned unparseable output, the tool ran forever:
never coaching, never erroring, reporting success the whole time.
proceed stays true - a coaching sidecar outage must not block anyone's
real work - but the response now carries degraded/error and a non-empty
text saying what failed, and the MCP tool renders that instead of the
success string. The helper moved to coach-degraded.ts so it can be
tested without booting an HTTP listener and a Postgres pool; eight tests
pin the contract.
TRAILHEAD_AUTO_CREATE_TEAMS now defaults OFF (index.ts:81). It defaulted
on, so any string any stranger sent as X-Team-Token silently provisioned
a real tenant row - unauthenticated tenant creation and an unbounded
write amplifier. Opt in with =true for open demo deploys.
Delete scripts/{list-team-prompts,delete-prompt,find-prompt-to-delete}.mjs.
Two embedded a live non-demo tenant token; all three were unreferenced
one-offs. NOTE: the token remains in git history and MUST be rotated by
the repo owner - that needs their account and cannot be done from here.
Add MIT LICENSE (Copyright (c) 2026 Bogdan Truta). Its absence also hard
-blocks vsce package. Manifest license fields follow in a later commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Teams will not pour knowledge into a store they cannot get it back out
of. Until now there was no export path at all, which makes the wiki a
roach motel and a fair reason to refuse to adopt it. Export is both the
trust signal and the backup story.
GET /wiki/export returns the whole team wiki as one markdown document
(text/markdown, content-disposition attachment). ?drafts=true includes
draft learnings; ?format=json returns { filename, markdown } for browser
clients that want to trigger their own download.
The renderer (wiki-export.ts) is pure - nodes in, string out, injected
clock - so the output contract is covered by fourteen real tests rather
than a smoke test around a database. The case worth calling out: prompt
templates routinely contain their own ``` blocks, so the fence width is
computed from the longest backtick run in the body. A hardcoded three-
backtick fence closes early and silently truncates the export mid-
document, which is the kind of corruption nobody notices until they need
the backup.
The tree query moved to wiki-tree.ts so /wiki/tree and /wiki/export read
the same rows through one query instead of two copies that drift.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Read the actual query predicates in apps/api/src/index.ts against the indexes that existed: - `captures` had nothing beyond its primary key, yet GET /team/metrics filters it on (team_token, created_at) and the dashboard polls that every 30s per open viewer. Every poll was a sequential scan. - The /score dedup probe filters skill_observations on (team_token, user_id, dimension, prompt_hash, ts). The only index led with (team_token, dimension) and carried neither user_id nor prompt_hash, so it could not serve the probe - which runs once per dimension, five times per /score. - GET /skill-arc and the COUNT(DISTINCT user_id) in /team/metrics filter on (team_token, ts) and never on dimension, so they could not use that index either. - idx_nodes_team_token_path duplicated the UNIQUE (team_token, path) constraint on the same columns. Dropped. schema.sql:2 claimed "Six tables"; there are eight - the wiki_jobs and wiki_job_paths tables landed with rich bootstrap and the header was never updated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…duct Each of these was checked against the code, not against the spec: - Score card "debounced 250 ms hits to /score" and the "5-second nudge / auto-sends as-is on timeout" describe behaviour that was deliberately removed. send-intercept.ts:17-21 lists both as gone. The extension now scores on send and never auto-fires a prompt. - Model names: README claimed Gemini 2.5 Flash and Gemini 2.5 Pro. packages/scoring/src/models.mjs uses gemini-3-flash-preview (score, topic, diff) and gemma-4-31b-it (extract). Nothing uses 2.5 Pro. - "Four hero tools" - tools.ts registers five (coach, wiki_lookup, wiki_save, wiki_bootstrap, wiki_proven_prompts). Added the missing row. - Cmd+Shift+K (vscode-ext/README.md:12): the manifest contributes no keybindings at all and one command, trailhead.refresh. The articulation scaffold was deferred during the original build and never written. Moved to an explicit "specced but never built" section. - dashboard/README.md:47 "All four routes prerender as static": there are five, and / is force-dynamic. - browser-ext/README.md:47 pointed at PINNED_CHROME.txt, which was never created and does not pin anything. Also noted in the MCP section that npx trailhead-mcp does not work - the package is private:true and neither name exists on npm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- c.txt was five space characters and referenced nowhere. - pitch-before-after.html (28 KB) is a one-off pitch artifact, unreferenced by any build or doc. Moved next to the other one in archive/ rather than deleted, since it is presentation history. Not done here: apps/landing-page/assets/logo-full.png is 488 KB and could be an order of magnitude smaller, but re-encoding a brand asset without the owner eyeballing the result is not a call to make from a script. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ExamplesItem/ExamplesResponse and WikiRecentItem/WikiRecentResponse were
re-declared locally in both apps/mcp-server/src/api-client.ts and
apps/vscode-ext/src/api.ts, while the neighbouring types in the very same
import block came from @trailhead/shared. The local copies were
field-identical to the server's definitions but had no compile-time link
to them, so a server-side change would have typechecked cleanly on both
sides and failed only at runtime.
Both files now import the shared definitions and re-export the names, so
existing importers are unaffected.
Also added SearchItem/SearchResponse to shared and applied SearchResponse
to the GET /search handler, which previously returned c.json({ items: rows })
with no declared contract at all - the one endpoint whose response shape
nothing was checking.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
packages/scoring ships ten hand-written .d.mts declarations next to the .mjs implementations they describe. Two gaps: 1. tsconfig include was ["src/**/*.ts"], which matches neither .mts nor .mjs, so with no allowJs/checkJs the implementations were never typechecked by anything. Enabled allowJs + checkJs and widened the include. It found real implicit-any in the new test, which is the point. 2. Nothing compared a declaration to its implementation. TypeScript resolves importers to the .d.mts and never looks at the .mjs, so a declared export that does not exist gives every consumer `undefined` at runtime while the whole repo still typechecks green. declarations-match.test.mjs parses the value-level exports out of each .d.mts, imports the .mjs, and asserts the two sets match in both directions. All ten pairs are currently in sync. Verified it actually fails: adding a phantom `export declare const PHANTOM_MODEL` to models.d.mts turns it red with a message naming the file and the symbol. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The public marketing page loaded react.development.js and react-dom.development.js - several times the size of the minified builds, running every dev-only invariant and warning path for every visitor. Documented the two limitations this does not fix: @babel/standalone still compiles the JSX in-browser on every load, and cdn.tailwindcss.com is a dev-time CDN Tailwind tells you not to ship. Both need a real build step, which also turns the Vercel deploy from "serve static files" into "run a build" - out of scope for this pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pairs with the MIT LICENSE added earlier. apps/vscode-ext and apps/mcp-server follow in the next commit - they are being edited concurrently for the self-hosting change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
classifyBubble returned a role when a hint selector matched the node ITSELF *or* anything in its subtree (node.querySelector(sel)). The content script walks outermost-first and deliberately lets the outermost match win, so the first wrapper div that happened to contain a user message classified as a user bubble and swallowed the entire thread - every per-message widget (score badge, outcome rating, prompt diff) then mounted once, on the wrong element. classifyBubble now matches on the element itself only. walkBubblesIn queried every `div, article, li` in the subtree and asked each one. On a long conversation that is thousands of elements per mutation batch - i.e. on every streaming token. It now queries the hint selectors directly via BUBBLE_HINT_SELECTOR: same results, a fraction of the work, and no ambiguity about which element in a nesting chain is the bubble. Nine tests cover it, including the exact regression: a container that merely contains a user message, and a thread container holding both roles, must both classify as 'unknown'. classifyBubble only calls matches(), so the tests use a stub element rather than a DOM. (content.ts also picks up the initApiUrlState() call from the self-hosting change landing alongside this.) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…efault
## GET /teams
The endpoint was unauthenticated, returned every team on the server
together with its token, and CORS is `*`. The team token is the only
credential this system has - it grants read on the wiki (which
summarises private source code) and write on everything - so one GET
from any web page compromised every tenant at once. What paid for that
was the browser popup's convenience of pre-filling a team dropdown
before any token was configured.
/teams now requires X-Team-Token and returns only the caller's own team,
as { name, id } where id is a truncated SHA-256 of the token: opaque,
stable, safe to render, and not replayable as a credential
(index.ts:127, 1557-1567 in the old numbering). TeamSummary no longer
carries `token` at all, so the type system enforces this at every call
site.
Client consequences, all deliberate:
- The popup's pick-a-team list is gone; switching teams means entering
that team's token, which the popup then resolves to a display name via
the authenticated endpoint. Adopting a team you don't hold a token for
is no longer possible, which is the point.
- The popup's reachability probe moved from /teams to GET /, the actual
unauthenticated status endpoint.
- The dashboard shows the team its own NEXT_PUBLIC_TEAM_TOKEN resolves
to, and no longer prints the token into the page.
## Self-hosting
trailheadapi-production.up.railway.app is deleted and returns 404, and it
was the hardcoded default in ten source files, so every client shipped
pointing at a dead server. Rather than re-point at another host that can
die, self-hosting is now the default: every surface defaults to
http://localhost:3000 (the port apps/api actually listens on) and says so
by name when it cannot reach it. The browser popup grows an "API server"
row that shows and edits the URL.
docker-compose.yml + apps/api/Dockerfile + SELFHOSTING.md bring up
Postgres and the API together, schema auto-applied on first boot, so a
stranger needs only a Gemini key. Both published ports bind to 127.0.0.1
deliberately: compose sets TRAILHEAD_AUTO_CREATE_TEAMS=true so
`trailhead-mcp init`'s derived per-repo token is accepted, and that
combination must not be reachable from the network.
## Also
- license: MIT + repository on the last two manifests (all nine now).
- contributes.viewsContainers used the "$(rocket)" codicon where VS Code
requires a file path, which fails vsce package. Added a real SVG.
- Dated roadmap/spec docs still cite the dead host; two contain
copy-pasteable config, so all three now open with a note saying the
host is gone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eams The /teams row still advertised "List all teams with tokens (unauth)" - now an accurate description of a fixed vulnerability. Also documents the markdown export endpoint and replaces "pre-allowlists the deployed Railway API" (which is deleted) with the localhost default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ams client The "npx trailhead-mcp does not work" disclaimer had been added in one place (README mcp-server section) but three other spots still told users to run it: the quick-start (cd into a target repo, then npx trailhead-mcp init), the deploy list, and the stack summary. The package is private/unpublished, so npx from a target repo hits the registry and fails. Replaced all three with the truthful clone-based invocation (node apps/mcp-server/bin/cli.mjs init), and corrected the stale "home page lists every team" line to match the now-authenticated, own-team-only endpoint. Also removed the dead listTeams/fetchListTeams from the dashboard client: nothing calls it (the home page does its own authenticated fetch), it sent no X-Team-Token so it would 401, and its comment still claimed the teams endpoint needs no auth -- the exact falsehood this PR set out to fix. Corrected the file header comment to match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the LearnLoop findings from
repo-audit.html. Every claim below was verified by running the thing, not by reading it.Verification (all run on this branch at final commit):
npm cinpm run typechecknpm testnpm run build --workspace=apps/dashboard✓ Generating static pages (7/7)Blocker fixes
tracedGeneratecalled itself —apps/api/src/gemini.ts:57invokedtracedGenerate(params)instead ofai.models.generateContent(params). Every LLM call unwound intoRangeError: Maximum call stack size exceededbefore any network I/O, so all ten exported functions were dead and/score,/coach,/diff,/improvewith them. The recursion was type-valid, which is whytscand CI stayed green.apps/apihad notestscript and zero tests — precisely why this shipped. Addedapps/api/src/gemini.test.ts: seven tests that stubglobalThis.fetchand assert a request actually reaches the transport, which is what infinite recursion prevents. Verified as a real regression guard: reintroducing the self-call turns the suite red withRangeError, not merely a failed assertion.Two defects in the pre-existing uncommitted work were found and fixed before building on it: the test asserted five dimension names that do not exist (
constraints/output_shape/verificationvs the realspecificity/constraint_articulation/output_specification), so 3 of 7 tests failed; andapps/api/package.jsonpointedtestat two files that did not exist. It had never been run./coachfailed open silently —apps/api/src/index.ts:471-483answered a scoring failure with{ proceed: true, text: '', overall: 0 }. The MCP tool (apps/mcp-server/src/tools.ts:339) renders an emptytexton aproceed: truescore-mode response as(coach overall: 0/10 — no coaching needed)— byte-identical to a flawless prompt. Whenever Gemini was down the tool ran forever: never coaching, never erroring, reporting success.proceedstaystrue(a coaching sidecar outage must not block real work), but the response now carriesdegraded/errorand a non-emptytextnaming the failure, and the MCP tool renders that instead of the success string. Helper extracted toapps/api/src/coach-degraded.tsso it is testable without booting an HTTP listener and a Postgres pool; 8 tests pin the contract.No LICENSE, no
licensefield in any of nine manifests — also hard-blocksvsce package. Added MITLICENSE(Copyright (c) 2026 Bogdan Truta) andlicense: "MIT"to all nine manifests, plusrepositoryon the two publishable ones.The dead production host —
trailheadapi-production.up.railway.appreturns 404 and was the hardcoded default in ten source files. Rather than re-point at another host that can die, self-hosting is now the default: every surface defaults tohttp://localhost:3000(the portapps/apiactually listens on) and names the exact env var / setting when it cannot reach it. Changed:apps/browser-ext/src/config.ts:3,apps/vscode-ext/package.json:44,apps/vscode-ext/src/extension.ts:19,apps/dashboard/src/lib/api.ts:20,apps/mcp-server/bin/cli.mjs:56,apps/mcp-server/src/reset-cli.ts:67,apps/mcp-server/src/bootstrap-cli.ts:164,apps/mcp-server/src/smoke-test.mjs:28,apps/mcp-server/src/verify-all-tools.mjs:14,apps/browser-ext/manifest.json:10,apps/browser-ext/scripts/smoke.sh:8.The browser popup gains an API server row that shows, edits and probes the URL. The dashboard fails at request time (never at module scope) so
next buildstill produces its 7 routes.Security
GET /teamsreturned every tenant's token, unauthenticated, with CORS*(apps/api/src/index.ts:127,1557-1567). That token is the only credential in the system — read on the wiki (which summarises private source code) and write on everything — so one GET from any web page compromised every tenant at once.It now requires
X-Team-Tokenand returns only the caller's own team, as{ name, id }whereidis a truncated SHA-256 of the token: opaque, stable, safe to render, not replayable.TeamSummaryno longer carriestoken, so the type system enforces this at every call site.Client consequences, all deliberate: the popup's pick-a-team list is gone (switching teams means entering that team's token, which the popup resolves to a display name); the popup's reachability probe moved to
GET /, the genuine unauthenticated endpoint; the dashboard shows the team its ownNEXT_PUBLIC_TEAM_TOKENresolves to and no longer prints the token into the page.TRAILHEAD_AUTO_CREATE_TEAMSdefaulted on (apps/api/src/index.ts:81) — any string any stranger sent silently provisioned a tenant row. Now defaults off; opt in with=true.A live tenant token was committed in
scripts/list-team-prompts.mjs:18andscripts/delete-prompt.mjs:18. All threescripts/*.mjswere unreferenced one-offs and are deleted. See Not done below — the token still needs rotating by hand.Compose ports bind to
127.0.0.1.docker-compose.ymlsetsTRAILHEAD_AUTO_CREATE_TEAMS=truesotrailhead-mcp init's derived per-repo token is accepted; that combination must not be reachable from the network, so neither published port listens on0.0.0.0.Honesty pass
Every claim checked against code, not against the spec:
README.md:99,108,320-323— the debounced 250 ms scoring, the 5-second nudge and auto-send-on-timeout were all deliberately removed;apps/browser-ext/src/send-intercept.ts:17-21lists them as gone. The extension scores on send and never auto-fires.README.md:84-85,288,371— claimed Gemini 2.5 Flash / 2.5 Pro.packages/scoring/src/models.mjsusesgemini-3-flash-previewandgemma-4-31b-it. Nothing uses 2.5 Pro.README.md:123— "Four hero tools";apps/mcp-server/src/tools.tsregisters five. Added the missingwiki_proven_promptsrow, plus a note thatnpx trailhead-mcpdoes not work (private: true, neither name on npm).apps/vscode-ext/README.md:12—Cmd+Shift+Karticulation scaffold. The manifest contributes no keybindings at all and one command (trailhead.refresh). Moved to an explicit "specced but never built" section.apps/dashboard/README.md:47— "All four routes prerender as static". There are five app routes and none prerender:/isforce-dynamic, the other four readsearchParams. Only the framework's/_not-foundis static, which is why the build reports 7. (My own first correction here said "four are static" — caught it by reading the build output instead of trusting the audit.)apps/browser-ext/README.md:47— pointed at aPINNED_CHROME.txtthat was never created.packages/db/schema.sql:2— "Six tables"; there are eight (wiki_jobsandwiki_job_pathslanded with rich bootstrap).New features
Self-host docker-compose —
docker-compose.yml+apps/api/Dockerfile+.dockerignore+SELFHOSTING.md+ rewritten.env.example. Postgres 16 with a named volume andpg_isreadyhealthcheck, schema auto-applied on first boot viadocker-entrypoint-initdb.d, API gated onservice_healthy. A stranger needs only a Gemini key.Wiki export to markdown —
GET /wiki/exportreturns the whole team wiki as one markdown document (?drafts=true,?format=json). The renderer is pure, so 14 tests cover the output contract. The case worth calling out: prompt templates routinely contain their own triple-backtick fences, so fence width is computed from the longest backtick run in the body — a hardcoded three-backtick fence closes early and silently truncates the export, the kind of corruption nobody notices until they need the backup. The tree query moved toapps/api/src/wiki-tree.tsso/wiki/treeand/wiki/exportshare one query instead of two copies that drift.Remaining MEDIUM/LOW
apps/browser-ext/src/selectors.ts:133-141,content.ts:78-91) —classifyBubblematched the node or any descendant, and the walker went outermost-first and let the outermost win, so one wrapper div swallowed the whole thread and every per-message widget mounted on the wrong element. Now matches the element itself only. The walker also stopped testing everydiv, article, li(thousands of elements per streaming token) in favour of querying the hint selectors directly. 9 tests.apps/mcp-server/src/api-client.ts:19-43,apps/vscode-ext/src/api.ts:9-26) — four shared types re-declared locally while their neighbours in the same import block came from@trailhead/shared. Both now import and re-export. Also addedSearchItem/SearchResponseto shared and applied them toGET /search, which returnedc.json({ items: rows })with no declared contract at all..d.mtsdeclarations (packages/scoring/tsconfig.json) —includewas["src/**/*.ts"], matching neither.mtsnor.mjs, so nothing typechecked the implementations. EnabledallowJs/checkJsand widened the include (it immediately caught real implicit-any). More importantly, nothing compared a declaration to its implementation:declarations-match.test.mjsparses each.d.mts's value-level exports, imports the.mjs, and asserts both sets match in both directions. All ten pairs are in sync. Verified it fails: a phantomexport declare const PHANTOM_MODELturns it red naming the file and the symbol.packages/db/schema.sql:71-81,96) —captureshad nothing beyond its PK yet/team/metricsfilters it on(team_token, created_at)and the dashboard polls every 30s per viewer. The/scorededup probe filtersskill_observationson(team_token, user_id, dimension, prompt_hash, ts)but the only index led with(team_token, dimension)and carried neitheruser_idnorprompt_hash— and that probe runs once per dimension, five times per/score. Droppedidx_nodes_team_token_path, which duplicated theUNIQUE (team_token, path)constraint. Migration:packages/db/migrations/2026-08-17-hot-path-indexes.sql.contributes.viewsContainersused the$(rocket)codicon where VS Code requires a file path, which failsvsce package. Added a real SVG atapps/vscode-ext/media/trailhead.svg.c.txt(five space characters, unreferenced), movedpitch-before-after.htmlintoarchive/, switched the landing page fromreact.development.jsto the production builds.README.md:9-10already explains that Trailhead is the internal codename and LearnLoop the product name; renaming nine packages is a large, risky change for little gain.Not done and why
repo_dbab62ba8d72ca37is removed from the working tree but remains in git history, and history was not rewritten (out of scope, and it would break every existing clone). Until it is rotated it remains a valid credential for that team. This needs the owner's account — it cannot be done from here.docker compose configvalidates (exit 0 withGEMINI_API_KEYset, exit 1 with a readable message without it), so schema, interpolation, volumes and healthcheck wiring are checked — butdocker compose upwas never executed. Unproven at runtime: that the image builds, thatnpm cisucceeds in-container, that the API connects to Postgres, and that the schema auto-applies. These are reasoned from source, not observed. Please run it once before advertising it.npx trailhead-mcpstill does not work. The package remainsprivate: trueand unpublished; renaming, unprivating, building todist/and bundling the workspace deps is a publishing decision for the owner, not a code fix. The README now says so plainly instead of advertising it.vsceblocker is fixed, but producing icon art and store listings is a design/publishing task.@babel/standalone, and still uses thecdn.tailwindcss.comdev CDN. Both need a real build step, which also turns the Vercel deploy from "serve static files" into "run a build" — a bigger change than this pass, and documented inline inindex.html.apps/landing-page/assets/logo-full.pngis 488 KB and could be an order of magnitude smaller. Re-encoding a brand asset without the owner eyeballing the result is not a call to make from a script.apps/landing-pageis not actually a workspace (it has nopackage.json) despite the rootworkspaces: ["apps/*"]implying it. Left alone; changing it affects the deploy.🤖 Generated with Claude Code