Repository navigation
fix: boot-bound request signatures, per-client limits behind a proxy, and a stricter gate (review E-01, E-02, E-03, E-06, E-07) - #38
Merged
Conversation
Security review E-07. The router let a request through without the token when its path_info was ["health"] or ["api", "v1", "proof"]. path_info drops empty segments, so //api/v1/proof, /api/v1/proof/ and GET /health/ were open too, although the HTTP API reference says only the exact paths are. The check now compares the raw request path, so every other spelling needs the token (401), and a test sends each spelling. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ayla Croft <dev@scriptkittyos.com>
Security review E-03. Some providers take their key in the query (?key=...), and the start-up summary logged LLM_URL whole at info. The summary now shows a service address without user name, password, query or fragment (https://host/v1?…), and the values in the queries of LLM_URL and TRANSCRIBE_URL, as written and decoded, join the secrets the log filter takes out of every line. Queries are still allowed (some providers need ?api-version=), and the model client now puts the request path before the query instead of after it, so such an address works. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ayla Croft <dev@scriptkittyos.com>
…headers Security review E-02. Requests a reverse proxy on this computer forwards were counted together for the wrong-token lockout and the proof limit, so a stranger sending ten wrong tokens a minute kept the owner's other devices out for as long as they kept at it. And a proxy that added only a header outside the list (Via, X-Client-IP, X-Cluster-Client-IP, Fastly-Client-IP, X-Original-Forwarded-For, ...) made its clients look like this computer's own, which are never limited. The list now covers those headers, any X-Forwarded-* and Tailscale's Tailscale-User-*. The new opt-in setting TRUSTED_PROXY_HEADER (x-forwarded-for, x-real-ip, cf-connecting-ip or forwarded; checked at start) names the header the proxy sets; each proxied client is then counted by that header's address (the rightmost X-Forwarded-For entry, the last Forwarded for=), normalised like a peer. Unset, the shared count stays. The reference no longer says the extension is never locked out behind a proxy: only the extension on this computer is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ayla Croft <dev@scriptkittyos.com>
…t replay Security review E-01. The server remembers the nonces of signed requests only in memory, and refused only a ts from before its start. So a request a program caught while the server was stopped could be played to the server after a restart when it was stamped ahead of the server's clock (a client clock up to 119 s fast, or the start second): reviewer E got 200 and the learner's words with ts +30, +90 and +119 s. The server now makes a random boot id (16 bytes, base64url) at each start. The proof answer carries it with boot_mac = HMAC(token, "kotiko-boot-v1\n" + nonce + "\n" + boot), and every signed request signs it: the header is now Kotiko-HMAC v2 with a boot= field, and the canonical request (kotiko-req-v2) has the boot id between the nonce and the body hash. A request for another boot id gets 401 error="stale_boot" (not counted as a guess, since its MAC matched); the extension then proves again and sends the request once more. The window stays 120 s either way; the start-time check is gone, since the boot id covers it without refusing a slightly slow client clock just after a start. The HTTP reference has the new scheme, the clock requirement and new vectors, checked by the server's and the extension's tests; a full-stack test replays reviewer E's scenario on the real server across a restart. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ayla Croft <dev@scriptkittyos.com>
Security review E-06. The store gate still read 14 disguised open lines beside the exact closed line as closed: "1. Gate: open", "| Gate | open |", inline code, "~Gate: open~", an emoji or arrow before it, "<b>Gate</b>:", "Gate:", "$Gate$:", a fake "## Appendix" inside an HTML comment or a code fence (so the open line below it counted for nothing), a heading followed by a zero-width character, a closed line only inside a comment, and "Status: the review is still OPEN" prose. A line now counts as a gate line if any reading of it, or of it joined with the next line, has the gate word (lookalikes as before) followed within a few non-letters by a colon-like or table-cell separator. The readings take out HTML comments (across lines too), tags and entities, and normalise as before; text in comments and code still counts, the safe side. Prose that says "open" beside "gate", "review" or "status" keeps the gate open. The appendix heading is the first visible one in plain ASCII, and the closed line must be visible (not in a comment, code fence or HTML block) and in a paragraph without strikethrough, HTML, math, code or entity markup. Reviewer D's cases and the earlier tests keep their results; reviewer E's sixteen are in the unit tests, with more of the same kind and a report shaped like the real one that must still close. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ayla Croft <dev@scriptkittyos.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL (js/incomplete-multi-character-sanitization) flagged the single pass that removes HTML tags before reading a gate line: "<<b>b>" leaves "<b>". The script only reads report text and never renders it, but a tag left over could hide a gate word from the check, so the removal now repeats until the line stops changing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ayla Croft <dev@scriptkittyos.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ayla Croft <dev@scriptkittyos.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.
Fixes E-01, E-02, E-03, E-06 and E-07 from reviewer E on
v1.0.0-rc.3(slice 54, SCR-476). Each comes with regression tests; the builder replayed reviewer E's proofs against live servers built from the old code and from this branch.boot_mac), and the extension signs it into every request (Kotiko-HMAC v2 … boot=…). A request carrying an old boot id gets401 stale_boot, so the extension proves again and retries once. Timestamps more than 120 s in the future are refused too. Reviewer E's replay after a restart: before, +30/+90/+119 s answered200with the word list; now every case gets401 stale_boot. New test vectors are inhttp-api.md, computed independently in Elixir and JS.Via,X-Client-IP,X-Cluster-Client-IP,Fastly-Client-IP,X-Original-Forwarded-For, anyX-Forwarded-*and others. New opt-inTRUSTED_PROXY_HEADER(x-forwarded-forrightmost,x-real-ip,cf-connecting-iporforwarded) counts each proxied client separately, so a stranger can't lock out the owner's other devices. The docs that said "never the extension" are corrected.?…, and the query values go to the log redactor. Queries still work, so?api-version=addresses are fine, and the model client now puts the path before the query.--dir)./healthandPOST /api/v1/proof.Compatibility: an rc.3 extension and this server don't talk (
malformed), and this extension with an rc.3 server asks you to update the server. Update both together.Checks: format, compile with warnings as errors, credo strict,
mix test1205 tests and 23 properties with 0 failures, Sobelow; lint; coverage 94.39 % lines, 85.04 % branches; Playwright loopback, fullstack, connect and security 21/21; REUSE; DCO.🤖 Generated with Claude Code