Repository navigation
Close the admin takeover through the delete-me link - #2262
Merged
Merged
Conversation
The admin approval page wrote the server's response and error into innerHTML, and both can carry values controlled by the user who asked to be deleted, so markup in them ran as script in the admin's session. The page now builds the heading and the details as elements and sets their text, so nothing the server returns is parsed as HTML.
A delete-me token names the user, is signed like a session token and lives for three months. The comment beside its delete_me attribute said it could not be used to log in, but nothing checked it, so whoever held the link could act as that user. The auth validator now refuses claims carrying delete_me. The admin deletion handler parses the token itself and keeps accepting it.
The site in a token comes from the login request, and nothing compared it with the configured sites, so any value got a signed session and was echoed back in responses and error messages. With the bolt store the token service now issues and accepts tokens only for the sites given in --site. A remote store decides on its own side which sites exist, so with one any audience is still accepted. The delete-me page fell back to the default site, remark, which an instance not serving it now refuses. Deletion links carry site_id, the signed-out message sends the admin to the requesting site's comments, and the API docs list the site parameter the admin endpoint requires.
size-limit report 📦
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2262 +/- ##
==========================================
+ Coverage 74.01% 74.56% +0.54%
==========================================
Files 129 129
Lines 3745 3751 +6
Branches 830 865 +35
==========================================
+ Hits 2772 2797 +25
+ Misses 967 948 -19
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
Strict TypeScript compilation fails, and site/audience authorization remains insufficiently bound.
3 open findings
What changed in this PR
Hardens the delete-me workflow against script injection and token misuse.
Changes:
- Renders deletion responses safely as text.
- Restricts delete-me tokens and Bolt-store audiences.
- Includes the requesting site in deletion links and adds regression tests.
| File | Description |
|---|---|
site/content/docs/contributing/api/index.md |
Documents the required site parameter. |
frontend/apps/remark42/app/deleteme.ts |
Safely renders deletion-page content. |
frontend/apps/remark42/app/deleteme.test.ts |
Tests injection-resistant rendering. |
backend/app/rest/api/rest_private.go |
Adds the site to deletion links. |
backend/app/rest/api/rest_private_test.go |
Updates link expectations. |
backend/app/cmd/server.go |
Rejects deletion tokens as sessions and restricts audiences. |
backend/app/cmd/server_test.go |
Tests token and audience restrictions. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
CI installs the newest Go 1.27 patch release, now 1.27.2, and golangci-lint v2.13.2 cannot read its export data, so the lint step failed with an export data decoding error. v2.14.0, built with Go 1.27.1, reports no issues in backend/app and backend/_example/memory_store under Go 1.27.2.
Admin rights hold for the site the session belongs to, but the admin deletion endpoint deleted from whichever site the token named, so an admin of one site could process a deletion request for another site served by the same instance. The endpoint now refuses a token whose site differs from the request's site, which the site check has already matched against the session. The basic-auth admin, who administers every site, may still omit it. The handler test server now takes query sessions from the jwt parameter, as the server does, leaving token to the delete-me token.
The auth package matches audiences ignoring case, so with the bolt store and --site=remark a token for REMARK was issued and accepted, while store operations with it failed, as the store keys sites by their exact name. With the bolt store, sessions whose audience is not exactly a configured site are now rejected.
This was referenced Oct 11, 2026
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.
Previously, the delete-me approval page rendered values from the server response as HTML, and some of them come from the user who requested deletion, so markup in them could run as script in the admin's session. The delete-me token was also accepted as a session, the site in a session token was not checked against the configured sites, and the admin deletion endpoint processed a token for any site.
After this change:
--site, and sessions are accepted only when the site matches exactly, since the auth package compares case-insensitively at issue. Remote-store deployments keep accepting any audience, as the remote store decides which sites exist.siteparameter of the admin endpoint.Regression tests cover the page rendering, delete-me token rejection, the site binding of the deletion endpoint, and audience restriction at issue, parse and login; each fails with its fix reverted. On a build serving only
--site=myblog, an admin processed a deletion through the shipped page in a browser.