Security review (October 2026)

Two deliberate passes over the code before it goes public and the site takes payments: the first for issue #57 (2026-10-17), the second on 2026-10-19 (issues #356–#395 and #429–#431, fixes merged 2026-10-21). It records what was checked, what was fixed, and what is accepted as a known risk, so the next review starts from here. The sections after the two Fixed tables describe main after both. How to report a problem: SECURITY.md.

Each fix has a test; the test names are given so they can be found with cargo test <name>.

Fixed in the first review (2026-10-17)

Fixed in the second review (2026-10-19)

Thirteen PRs, merged into main on 2026-10-21. Each row names the issue, the PR and its merge commit.

Migrations were renumbered when these PRs merged: #433’s users rebuild landed as v45 (its PR body says v42) and #436’s sms_sends ledger as v46.

Still open from this review:

• #379, PR #461 (registration never reveals whether an address has an account; password reset): not merged. Its “same answer for every address, no session” registration conflicts with course sharing’s invite link that registers and lands the learner in the shared course (#492), so it waits for a product decision.
• #356 (CI runners): blocked until the repository is public; see Accepted risks.

Checked and found sound

• Passwords: argon2id (8 MiB, 3 passes, per-user salt); unknown addresses verify against a dummy hash so timing does not reveal them; 8–128 characters.
• Session cookies: 256-bit random token, HttpOnly, SameSite=Lax, Path set, Secure with SKRONGLE_SECURE_COOKIES=1; fixed 30-day expiry, checked in SQL on every request, purged hourly; logout deletes the row and clears the cookie; a new token on every login (no fixation).
• Google sign-in (src/oauth.rs): random state, PKCE (S256) and nonce kept in a 10-minute HttpOnly cookie and checked; the redirect URI is fixed from SKRONGLE_PUBLIC_URL; issuer, audience, expiry, nonce and email_verified are checked. Accounts are matched by Google’s sub only, never by e-mail address, so a Google account with someone’s address cannot take over their password account. Linking needs a signed-in session and the state cookie of that browser.
• Private courses: every /courses/{id}… and /api/courses/{id}… handler that changes a course goes through owned() (404 or 400 for anyone but the owner); opening a course checks accessible() (built-in, owned, or shared with the account, #451), and Project checks the same again on every request (#383), so every learning route and /pictures only ever loads a built-in course, one the user owns, or one shared with them (read-only). A share by e-mail address matches only an account that has confirmed that address. Made themes are owner-only (made_themes_are_private_over_http).
• Generation jobs: /api/generate/{job} (and the old /courses/generate/{job} redirect) check the job’s user (job_of), and the editor’s modal shows only the owner’s job on their own course; cancel, resume, undo and AI changes go through owned().
• Progress API (/api/progress…): agents use the learner’s own session cookie; every handler takes Project, so it reads and writes only that user’s rows in their active course.
• In-memory sessions: tutor chats and timed games are looked up by id and user id.
• Stripe: the webhook checks Stripe-Signature (HMAC-SHA256, constant-time compare, 5-minute tolerance) before parsing the event; the plan changes only through it; the secret key stays on the server; no secret is in templates or JS.
• Learner API keys (src/secrets.rs): ChaCha20-Poly1305 with a fresh nonce, user id and provider as associated data, master secret from SKRONGLE_SECRET_KEY or a 0600 file created with create_new; never logged, never sent back (last four characters only). Keys pasted for one job live in memory only.
• Uploads and the editor: paths must be course.json, lessons/<name>.json or pictures/<name>.svg (no .., no leading slash, a safe character set, 120 characters); 1 MiB per file, 400 files per course, 100 per upload, 8 MiB per course and 32 MiB per account in all, 50 courses per account, 120 course writes per 5 minutes (#366); request bodies are capped at 64 KiB, 2 MiB only on the session-gated editor and AI routes and the dev tools, 256 KiB on the Stripe webhook (#370); JSON parsing is serde_json with its recursion limit (128). AI-written courses go through the same per-file and file-count caps (#367). SVGs are served with default-src 'none' and nosniff, and shown through <img>.
• Templates: askama escapes by default. The |safe uses are: lesson and stage Markdown (now sanitised, above), the Sprint and Listening session JSON inside <script type="application/json"> (routes::script_json escapes <, >, &, U+2028 and U+2029, #384), and /docs (the repository’s own files). All of static/*.js was read for innerHTML/insertAdjacentHTML sinks: every esc() escapes & < > " ', and apart from the Sprint label above every course or user value is escaped; there is no eval or new Function. Tutor replies are escaped before the small bold/italic formatting.
• Open redirects: the redirects that take a target from the request (/terms/accept?next=, /dev/login?next=) accept only an app path through terms::is_app_path: it starts with one / and has no \, whitespace or control character, so a tab cannot turn it into //evil.example (#358). Sign-in after a share link lands on a path the server builds from the token; Google step-ups return to a path the server chose when it started them. One weaker check remains: the language picker’s POST /lang back refuses // but not /\ or a tab, so it could send the browser off-site; a cross-site form cannot post it (the CSRF check), so only a same-site page could use it.
• Codes and links at rest: sign-in, two-factor and phone codes and the verification and export links are stored only as SHA-256 hashes (codes with a random salt). Their plain text exists only in the message itself: a mail_outbox or sms_outbox row keeps it while queued and loses it (and a code mail its subject) once sent or failed, and it is never logged or exported (#361, #429). An authenticator app’s secret is sealed like learner API keys.
• Prompt injection: pasted material is wrapped in <material> tags and the model is told to treat it as content. What a hostile file can still make the model write is lesson text: wrong or misleading content, or an http(s) link. It cannot add script (Markdown is sanitised, SVGs cannot run), cannot reach another account (the job writes only the owner’s course), and every file is validated against the course format before it is saved.

Accepted risks

• Superusers act on any account (#220). The only action so far, Reset email verification, unconfirms an account and e-mails it a new link. Until the owner clicks it, the account cannot add a phone number or turn on e-mail as its second step (#431), and courses shared to its address do not open for it; sign-in, including an e-mail second step already on, keeps working. So the harm is an unwanted e-mail and a temporary loss of those features. The flag is set only by hand (or on the dev user in dev mode), every action is logged with both user ids, and anyone else’s POST is a 404. Actions added there later need the same review.

• DNS rebinding on custom base URLs: the host is checked when the form is submitted and resolved again by the HTTP client when it connects, so a DNS server that changes its answer in between could still reach an internal address. Closing it needs a resolver hook in ureq (its API for that is unstable). The hosted site can also block it at the network level.

• CSP allows inline scripts ('unsafe-inline'): the templates still have three inline <script> blocks and a few onsubmit/onchange attributes. The policy still blocks scripts from other origins, framing, plugins and <base> tricks; XSS is prevented at the sinks (above). Moving the inline code into static/ would allow script-src 'self'.

• Secure cookies and HSTS are opt-in in Skrongle mini (SKRONGLE_SECURE_COOKIES=1), because a Pi on a home network is usually reached over plain HTTP. The startup warning says when to turn it on. The hosted site refuses to start without it (#980); both deploy targets serve HTTPS.

• Rate limits live in memory: they reset on restart and are per process. Behind a proxy that does not set X-Forwarded-For every client shares one address bucket (the per-account limit still applies).

• Proxy sign-in trusts the proxy completely (SKRONGLE_PROXY_AUTH_*, #642, off unless both are set): a request whose TCP peer is in SKRONGLE_PROXY_AUTH_FROM is signed in as whatever e-mail address its header names, with no password, two-factor step or rate limit, and in open mode an unknown address gets a new account. The peer is the socket’s own address, never X-Forwarded-For; the header from anywhere else is ignored and logged. So anything that can connect from a trusted address (every local program when it is 127.0.0.1) can sign in as anyone, and a proxy that passes on a client’s own copy of the header lets the client choose. Self-hosting says to bind to loopback and have the proxy overwrite the header; a range wider than /8 is warned about at startup. /dev/* is never signed in this way. It is a Skrongle mini feature: the hosted server refuses to start with either setting (#980).

• Root on the server can read saved API keys (the database and the key file are on the same box), as src/secrets.rs says.

• Dev mode (SKRONGLE_DEV=1) creates a dev@example.com / dev-password account and must never run against a real database. Besides the loopback-bind, secure-cookie and SKRONGLE_PUBLIC_URL guards above, it is now refused on a database that holds other accounts unless SKRONGLE_DEV_FORCE=1, and in dev mode every route answers 421 to a Host that is not this machine (#388). That makes it hard to switch on by accident in a deploy, not impossible: SKRONGLE_DEV_FORCE=1 copied into a deploy’s environment would do it.

• CI runners (#356, open): pull-request jobs run as root on the self-hosted runners beside a repository-admin token, on persistent shared volumes. While the repository is private only the owner opens pull requests; it must be fixed before the repository goes public, and is blocked until then.

• Registration is open to anyone who can reach the server (documented in self-hosting); there is no CAPTCHA. E-mail addresses are verified by a one-time link (src/verify.rs: 256-bit token, only its SHA-256 stored, 24 hours, single use, voided by a newer link or an address change; resend limited to 5 per account per hour, and at most 10 per address per day across account deletion, #395). Only a few things need a confirmed address: adding a phone number (or a Google account instead, #431), e-mail as the second sign-in step, and opening a course shared to that address. Otherwise an unconfirmed account can do everything a confirmed one can. Registration still says when an address already has an account, and there is no password reset, so someone who registers a victim’s address first keeps it: #379, open (PR #461, above).

• The verification link’s host comes from SKRONGLE_PUBLIC_URL, or, when that is unset, from the request’s Host (or X-Forwarded-Host). Without SKRONGLE_PUBLIC_URL someone registering with another person’s address through a forged Host header can make the link in that person’s mail point at their own site; if the person clicks it, the token leaks and the address can be confirmed on an account they don’t own. Set SKRONGLE_PUBLIC_URL on a public deploy (Google sign-in and Stripe need it anyway).

• Course and AI quotas are per account (#366, #367): 50 courses, 8 MiB per course, 32 MiB per account, 10 AI generations an hour and 40 a day. Registration is open, so many accounts can still add up; the server-wide AI slots and the hourly mail ceiling bound the expensive parts, not the disk.

Git history scan

gitleaks detect over the full history (121 non-merge commits) on 2026-10-17 reported four “generic API key” matches, all false positives: dummy values in a test (debug_output_never_shows_secrets), a sentence in this repository’s docs, and a constant name in static/dates.js. No API keys, tokens, private keys, .env files, database files or .key files were ever committed; the only env files are the *.example templates, whose secret variables are empty.

Personal data a fresh public history would want to drop or replace:

• Commit metadata: the author’s personal e-mail address and a machine-local address (<user>@<laptop>.local) on every commit. A fresh history (or a mailmap plus rewrite) should use a noreply address.
• Infrastructure names in old commits: the Pi’s public DuckDNS host name and its LAN address were in deploy/ and scripts/ (deploy.env.example, the nginx snippets, the systemd unit, deploy-env.sh, release.sh). They are not secrets, but they point at the owner’s home server. HEAD has placeholders since #390 (PR #439); the history still has them.

Rewriting history is the owner’s decision (TODO.md); this review did not.

Not done here

• Moving inline scripts and handlers into static/ so the CSP can drop 'unsafe-inline'.
• A resolver-level SSRF check (DNS rebinding, above).
• An automated browser test for the Sprint label escaping.
• Registration that does not reveal an address, and a password reset (#379, PR #461 open).
• CI runner isolation (#356).
• The language picker’s back check (POST /lang, above) should use terms::is_app_path.