Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2264 +/- ##
==========================================
+ Coverage 74.56% 74.98% +0.41%
==========================================
Files 129 129
Lines 3751 3754 +3
Branches 829 829
==========================================
+ Hits 2797 2815 +18
+ Misses 948 933 -15
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:
|
size-limit report 📦
|
There was a problem hiding this comment.
🟡 Changes recommended
Logout rejection still leaves UI state uncleared, and another Go 1.27 lint workflow remains on an incompatible version.
2 open findings
What changed in this PR
Fixes /web demos for non-default sites while preserving valid sessions on 403 responses.
Changes:
- Substitutes the first configured site into demo pages and cache validators.
- Retains sessions on 403 and clears credentials during logout.
- Updates release checks, tests, and backend lint tooling.
| File | Description |
|---|---|
scripts/prepare-release-assets.sh |
Validates demo site literals. |
frontend/apps/remark42/app/components/auth/auth.api.ts |
Clears credentials on logout. |
frontend/apps/remark42/app/components/auth/auth.api.spec.ts |
Tests logout cleanup. |
frontend/apps/remark42/app/common/fetcher.ts |
Retains sessions on 403. |
frontend/apps/remark42/app/common/fetcher.test.ts |
Tests 401/403 handling. |
backend/app/rest/api/webfiles.go |
Substitutes demo site IDs. |
backend/app/rest/api/webfiles_test.go |
Tests substitution and ETags. |
backend/app/rest/api/rest.go |
Configures site-aware web serving. |
backend/app/rest/api/rest_test.go |
Updates file-server tests. |
backend/app/cmd/server.go |
Selects the demo site. |
backend/app/cmd/server_test.go |
Tests site selection. |
.github/workflows/ci-backend.yml |
Updates golangci-lint. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| */ | ||
| export function logout(): Promise<void> { | ||
| return authFetcher.get('/logout'); | ||
| return authFetcher.get<void>('/logout').finally(dropSession); |
Collaborator
Author
There was a problem hiding this comment.
Fixed in a9dc97a: logout() now ignores the request failure once the session is dropped, so the signout action goes on to clear the signed-in user and the profile resets its signing-out state. The logout test now expects resolution, and a new store test checks that signout sets the user to null after a refused logout.
The build emits the demo, counter and last-comments pages for site remark. The docker image replaces that with the first configured site at container start, but the binary served them as built, so on an instance serving another site the demo's API requests failed with "site not found" and its widget asked about a site the reader was not signed in to. The file server now fills in the first configured site the same way, in the html files the docker image rewrites, and the site is part of the cache validator so a changed SITE is not answered with 304. The release build fails if those pages lose the site_id:"remark" literal the server replaces and the docker image's pattern expects.
The fetcher cleared the auth cookies on 403 as well as 401. The server answers 403 to a valid session that may not do something, such as one issued for another site served from the same remark42 host, an anonymous user, or a non-admin. Clearing the shared XSRF cookie then could sign the reader out of the other sites using those cookies too. Cookies are now cleared only on 401, which is what an invalid session gets. Logout is the one place a 403 can mean the session is over: an expired token sent as a header is refused there, so logout now forgets the local session whatever the server answers.
The logout request dropped the local session and then rejected, and the signout action clears the signed-in user only after it resolves, so a refused logout, such as an expired header token answered with 403, left the widget showing the reader as signed in without credentials. The request failure is now ignored once the session is dropped.
paskal
force-pushed
the
fix-web-demo-site
branch
from
October 11, 2026 09:56
a9dc97a to
6cdc1a2
Compare
Collaborator
Author
|
Rebased onto master. |
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 binary served the demo, counter and last-comments pages for site
remark, as the build emits them; only the docker image replaced that with the configured site. On any other site the demo's API requests failed with "site not found", and opening it signed the reader out of their real site: its widget asked for the user on siteremark, the server answered 403 because the session belongs to another site, and the frontend cleared the auth cookies on that 403.After this change, the file server fills the first configured site into those pages as the docker image does, and includes it in the cache validator. The frontend keeps the session on 403, since permission failures come with a valid session, and logout always forgets the local session, including when an expired header token makes it answer 403. The release build now checks that the demo pages keep the
site_id:"remark"literal the server replaces and the docker image's pattern expects.Tests cover the demo site substitution, cache invalidation, site selection, the session kept on 403 and the logout cleanup; each fails with its change reverted. A browser check of a build serving only
--site=myblogconfirmed that/web/used that site and kept the reader signed in.Resolves #2251.