6f7313fcd764e3072bad8b0316c97edffccb9005
3583
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Dmitry Verkhoturov
|
6f7313fcd7 |
Price the Storage Access route as it really stands: not reachable today
The section sold it as a permission prompt on top of the existing flow. A browser denies requestStorageAccess outright when the embedded origin has no recent first-party interaction to grant against, and remark42 never gets one: the reader interacts on the provider's origin, and the callback returns to a document whose first statement is window.close() under ?selfClose. So the real cost is changing the first-party experience, either by having the callback collect a click before closing or by establishing interaction some other way, which is a different order of cost from the other two routes. Also states the grant correctly. It lets the frame's requests carry the cookie; it does not make it script-readable, and the JWT cookie is HttpOnly regardless, which is what the OAuth paragraph above already says. |
||
|
Dmitry Verkhoturov
|
5224c5dfcf | Drop three more banned constructions from the R1 text | ||
|
Dmitry Verkhoturov
|
3b7a7778dc |
Price the Storage Access API, and stop calling the header path cookie-free
Two more from the review. The section declared that no frontend change could bring OAuth inside the criterion, which writes off the Storage Access API: an embedded frame can ask the browser, on a user gesture, for access to its own unpartitioned first-party cookies, and both Safari and Chrome implement it. That would read the cookie the callback already set with no backend work, at the cost of a revocable permission prompt. It belongs in the comparison, and it is the only one of the three routes that is not a backend feature. And the constraint bullet still offered CHIPS or a token not relying on ambient cookies as alternatives, when the header path relies on one as soon as the page reloads. Both routes rest on the same attribute; they differ over who writes the cookie and what that costs. |
||
|
Dmitry Verkhoturov
|
be61b5873a |
Correct the R1 mechanism: the attribute carries the reload, not the writer
A reviewer pass found the central claim inverted. The text said the token never travels as a third-party cookie because fetcher.ts writes it inside the frame. It does travel as one: activeJwtToken is a module-level variable filled only from the response header, nothing reads the JWT cookie back, and getCookie is called once in the whole app for XSRF-TOKEN, so the first request after a reload sends no header and the token arrives ambiently. What spares it from blocking is the Partitioned attribute authCookieOptions sets, which is what #2214's control cookie exists to prove. Consequences elsewhere in the section. Saying the upstream Partitioned work buys no flow that does not already work was false: AUTH_SEND_JWT_HEADER ships off, so in the default configuration nothing writes a partitioned cookie at all. The OAuth handoff was described only in the shape that needs that upstream work, when answering the redemption with X-JWT needs nothing upstream. The partition-key description was a counterfactual, since a cookie with no Partitioned attribute has no partition key, and the SameSite clause did not apply to a top-level callback navigation under AUTH_SAME_SITE=none. Also: the documentation gap is two edits, since the parameters page documents the flag but still promises SameSite=Strict and a __Host- prefix that #2197 removed; #2214 adds three TLS functions and only two are table-driven; the task list still claimed R1 has no e2e coverage; the constraint bullet still described setAuthCookie's pre-#2197 behaviour; and the Path B cost line had swapped the real upstream dependency for an invented one. Wrapped to 100 columns. |
||
|
Dmitry Verkhoturov
|
6831206574 | Drop two banned constructions from the R1 text and one from Path B | ||
|
Dmitry Verkhoturov
|
8e16a7373c |
Record that email is now measured, leaving Telegram as the inference
#2214 turned its two TLS cases into tables over anonymous and email, so both are exercised in a third-party frame with the reload and again under enforced partitioning. Telegram is the only one of the three still resting on the writer keying off X-JWT and not off the provider, with #2208 as the reason it cannot be measured and go-pkgz/auth#316 as what would change that. |
||
|
Dmitry Verkhoturov
|
864cde68c8 |
Narrow the R1 coverage claim to the flow the suite measures
#2214's two TLS cases both sign in anonymously, so anonymous is the only one of the three named flows exercised in a third-party frame. Email is covered over http and never embedded, and Telegram is covered nowhere and cannot be until #2208 makes the API base URL configurable. The expectation that all three behave alike rests on the client-side writer keying off X-JWT and not off the provider, which is an inference and now reads as one. |
||
|
Dmitry Verkhoturov
|
33e93a427d |
Correct R1 in the frontend plan: scope it to the flows that can meet it
The acceptance criteria promised sign-in with any configured provider under third-party cookie blocking, which OAuth as built cannot satisfy: the callback runs in a popup, a top-level context of its own, so the cookie it sets is keyed to the auth host and the frame embedded on the other domain is a different partition that never sees it. The criteria now name email, Telegram and anonymous, and OAuth carries its own paragraph explaining why it is out and what would bring it in, which is the server-mediated one-time code the constraints below already describe. Two claims went stale alongside it. Master meets the criterion today with AUTH_SEND_JWT_HEADER on, because fetcher.ts writes the cookie from inside the frame and the token never travels as a third-party cookie, so the remaining gap for those flows is documentation and not code. The server-set cookies still lack Partitioned, but nothing depends on them surviving in a third-party frame any more, so the upstream work they were waiting on buys no flow that does not already work. |
||
 Dmitry VerkhoturovandGitHub
|
3286f028e3 |
Document what each browser actually does with cross-domain auth (#2222)
Measured on real domains over real certificates, Remark42 on one registrable domain and the host page on another, with a control cookie behind every blocked column so a run that blocks nothing cannot report a pass. Three results the manual did not carry. Safari blocks third-party cookies out of the box, so AUTH_SAME_SITE=none on its own has already stopped working there, which makes the old recipe broken today and not deprecated later. Firefox reaches a working session by a weaker route than Chrome and Safari do: it accepts the server's attribute-less cookie, and because that cookie is HttpOnly the browser then forbids the widget's script from replacing it, so the session rides on an ordinary unpartitioned third-party cookie even with the header flag on. And Firefox's block-all setting discards partitioned cookies too, so no configuration survives it. Two parameter descriptions were wrong in ways that matter here. AUTH_SAME_SITE default emits no SameSite attribute rather than Lax, which is precisely what lets the widget's own cookie land on Chrome and Safari. And AUTH_TTL_COOKIE does not govern the cookie that carries the session under the header flag, since the frontend hardcodes 200h to mirror the default. |
||
 UmputunandGitHub
|
c947a06d48 |
Release the response before tearing the test server down (#2212)
TestRest_securityHeaders and TestRest_frameAncestors both start a server, read one response, and then call teardown() partway through the test to start a second server with different options. The first response body is only closed by a defer, which does not run until the test returns. httptest.Server.Close waits on connections still in use, so it blocks on a body that will not be closed until after it returns. The tests deadlock and the whole rest/api package dies on the timeout rather than on an assertion. CI pins go 1.25, where the responses are small enough that the connection goes back to the pool on its own and nothing hangs. On go 1.27 both tests hang, which is how this surfaced. Close the body and the client's idle connections before teardown() in both. |
||
|
UmputunandGitHub
|
5f439cf1d5 |
Qualify what works off-domain, and fix two typos beside it (#2221)
The opening summary said Telegram, Email and anonymous auth "would work everywhere". That holds only with AUTH_SEND_JWT_HEADER set, and the widget's own cookie is Secure, so the path is HTTPS-only and bounded by ALLOWED_HOSTS besides. The sentence now states those conditions. What happens without the flag is two separate things, whether sign-in succeeds in the frame and whether it survives a reload, and the body below already separates them. The other two are older: a stray backtick after "work on any domain", and "expect" for "except" in a bullet whose neighbour already says except. Related to #2218 |
||
|
Dmitry VerkhoturovandGitHub
|
7de51ad2ef |
Document what actually keeps a cross-domain reader signed in (#2218)
* Document what actually keeps a cross-domain reader signed in The separate-domain manual tells operators to set ALLOWED_HOSTS and AUTH_SAME_SITE and says authorisation then works anywhere. That stopped being true as browsers began blocking third-party cookies: the server-set auth cookies carry no Partitioned attribute, so a browser enforcing the block drops them whatever their SameSite value. What survives is AUTH_SEND_JWT_HEADER, where the token returns in a header and the widget writes its own partitioned cookie from inside the frame, and the manual never mentioned it. It now does, with the XSS trade-off and a pointer to the parameter page, and it says plainly that this rescues Email, Telegram and anonymous but not oAuth. The parameter page's own mitigation list was left wrong by #2197. It promised SameSite=Strict cookies and a __Host- prefix on HTTPS; authCookieOptions drops the prefix entirely and uses SameSite=None; Secure; Partitioned whenever the widget is embedded on another domain, which is the case the flag exists for. * Say that the JWT header is sent in addition to the cookies, not instead Both the flag's own help and the parameter table said the header replaces the server-set cookie. Service.Set does neither: it writes the header and then falls through to set both cookies, with a comment saying the cookies are needed because headers do not survive the OAuth redirect. An operator reading either description would expect the server to stop setting cookies once the flag is on, and would misjudge what the flag changes about their exposure. * Correct three details in the cross-domain documentation The link to the parameter page used Zola's @/ syntax, which Hugo emits literally as a relative href since there is no render-link hook. It was the only such link under site/content; the other manuals use the relative form and this now does too. The CHIPS description claimed Partitioned makes the cookie unreadable from any other page the browser visits. The partition key is the top-level site, so a different site gets a separate cookie while pages and subdomains under the same site share it. Overstating isolation on the page an operator reads to weigh risk is the wrong direction to be wrong in. And Chrome does not block third-party cookies by default: Google's April 2025 position keeps ordinary Chrome on user choice and names Incognito as the mode that blocks. Naming Safari, Chrome Incognito and browsers configured to block them says the same thing and stays true. * Drop AUTH_SAME_SITE from the recommended cross-domain recipe Measured rather than reasoned, because it reverses guidance this page has carried for years. With only the remark42-https service taken back to the default, both reload cases pass for anonymous and email, under a permissive browser and under one enforcing partitioning. The cookie jar after an anonymous sign-in says why. With the setting there are four cookies: the server's unpartitioned JWT and XSRF-TOKEN, and the widget's own partitioned pair. Without it there are two, the widget's pair alone, and the session behaves identically. So the setting is doing something real, which is what makes the passing run meaningful, and what it does is add an unpartitioned HttpOnly JWT delivered as a third-party cookie to every listed domain wherever the browser still permits that. Nothing needs it. It stays documented for the configuration that does need it, which is one without AUTH_SEND_JWT_HEADER, where the server's cookies are the only ones there are. One prediction the experiment falsified: the attribute case was expected to fail on the default server-set pair. It passes, because a cross-site Set-Cookie lacking SameSite=None is refused outright, so that pair is absent from the jar instead of present with the wrong attribute. The manual now says so. |
||
|
Dmitry VerkhoturovandGitHub
|
389189afcf |
Give each import request in TestMigrator_ImportDouble its own reader (#2220)
The test passed one strings.Reader as the body of both POSTs. client.Do returns once the response headers arrive, and the import answers 202 before the transport has finished copying the body, so the second http.NewRequest reads the reader's Len to set ContentLength while the first request's writeLoop is still advancing it. The race detector caught it on CI as a write in strings.(*Reader).WriteTo against a read in NewRequestWithContext, failing a test nothing had touched. Reproduced in isolation to confirm the mechanism rather than infer it from the trace: a handler that answers 202 without draining an 8 MiB body, two requests sharing one reader, and -race reports strings.(*Reader).Len in NewRequestWithContext against strings.(*Reader).Read on every run. It does not reproduce in this package locally, which is why it reads as a flake. Both requests now build their own reader over the same content. The second one carries a full body where before it inherited a consumed one, which is closer to what the case is about: a second import arriving while the first is running still has to be refused. |
||
|
Dmitry VerkhoturovandGitHub
|
6f40926241 |
Drop the origin-anchored public path from the delete-me bundle (#2219)
deleteme.ts set __webpack_public_path__ to window.location.origin plus /web/, which discards any path prefix the instance is served under. It is inert today because that bundle references no asset and loads no chunk, so the value is assigned and never read, but it is wrong by construction and would resolve at the domain root the moment anyone adds an image to that page. Removing it leaves webpack's own publicPath: 'auto', which derives the base from the script's URL and is right in both arrangements. |
||
|
Dmitry VerkhoturovandGitHub
|
2640aaee9e |
Reach what http cannot: the widget over TLS, embedded cross-origin (#2214)
Every service in the suite spoke http, and the browser gates a whole class of behaviour on the page protocol: Secure cookies, SameSite=None, Partitioned, and any code reading location.protocol. None of it was executed, which is how setAuthCookie came to decorate its cookies with __Host- on https pages and survive for years. A TLS pair joins the stack: remark42 with SSL_TYPE=static on 8443, and an nginx serving a host page on its own name on 8444, both on a self-signed certificate that e2e/tls/generate.sh makes and .gitignore keeps out. Every context accepts it, and so does the readiness client, since those are the only servers either talks to. The instance also runs with AUTH_SEND_JWT_HEADER, which is what makes the widget write cookies of its own: without it the client-side writer never runs on any https page here and every assertion about the attributes it chooses is vacuous. Three cases. Signing in across origins and then reloading, which is the one the http cross-origin case cannot make: the widget holds its token in memory for the life of a page, so signing in and posting says nothing about persistence and only the reload asks whether the cookie was delivered, stored under a name the backend reads and sent back from a third-party frame. The cookies themselves, read out of the browser store while the widget is embedded elsewhere, since a cookie the browser refused is absent from that list entirely and one it kept but will not send is worse than useless: every copy of both names has to be Secure and SameSite=None, at least one has to be partitioned, and none may carry a __Host- prefix nothing on either side reads. And the same reload under a browser that blocks third-party cookies, which the widget's own partitioned pair is the only reason to survive. That last one needs a browser playwright does not offer: its default arguments disable ThirdPartyStoragePartitioning outright, so a run configured wrongly keeps every third-party cookie and the case would pass while asserting nothing. IgnoreDefaultArgs drops that list and re-supplies it without the one feature, and a control cookie set from inside the frame has to be refused before anything else is read, so a playwright release that changes the list fails as itself instead of going quietly vacuous. All three pass against master. What TLS still cannot reach, the OAuth popup above all, is written down in the README. |
||
|
Dmitry VerkhoturovandGitHub
|
250e8ad925 |
Report the widget height when the sign-in panel closes (#2213)
The sign-in panel is positioned absolutely, so it grows the iframe without growing the document: `useDropdown` measures the panel itself and posts the sum, and a ResizeObserver on the panel keeps that number current while it is open. Closing it resizes no box anything watches. The panel observer goes with the element, and the document observer in `Root` sees nothing, because the document height never changed in the first place. Nothing then tells the parent to come back down, so the iframe keeps the open panel's height and the embedding page carries a hole under the widget for as long as the reader stays on it. The effect's cleanup now reports the height, with no element, so the number is the document's own. That is the one place both close paths reach: the click inside the widget, and the clickOutside message the host page posts when the reader clicks anywhere else. TestGeometry_HeightFollowsTheAuthPanelAndTheTextarea covers this and has been intermittently green: whether the frame comes back down without the fix depends on timing, and it fails on every run here while CI has been passing. The unit test fails with the cleanup reverted. |
||
|
Dmitry VerkhoturovandGitHub
|
4793c1cd2c |
Fill the instance URL into the embedded frontend at serve time, and stop pinning compressor output in tests (#2198)
* Assert what the image endpoints promise rather than the compressor's output Three tests pinned the exact bytes or the exact length of an encoded image, so they fail on any toolchain whose deflate or png encoder emits something different. CI pins go 1.25 and passes; go 1.27 fails all three, while the images themselves are perfectly valid. TestRest_QR now decodes both the golden file and the response and compares the pixels, which is the same assertion about the qr code and none about the encoder. The two resize cases assert the decoded image fits the box resize was given and touches one of its sides, which is what fitting to a box means and what the function actually promises. Resolves #2200. * Fill the instance URL into the embedded frontend at serve time The widget falls back to a compiled-in URL whenever a page omits `remark_config.host`. The bundler cannot know that URL, so it emits `{% REMARK_URL %}` and each distribution substitutes it: the docker image rewrites the files under its web root at container start, and the release binary, which serves the build embedded in itself, had nothing doing it. `prepare-release-assets.sh` filled the marker with `http://127.0.0.1:8080` before the embed instead, so every copy of the binary shipped pointing at the visitor's own loopback address, and on an https site the request is blocked as mixed content besides. It has been that way since v1.11.0, the first release to embed the frontend, and the earlier binaries embedded none, so the tarball has never served a correctly addressed widget. The placeholder now survives into the embedded copy and the file server fills it with the configured `REMARK_URL` as it serves, which is what the docker image already does to its own copy. The image no longer bakes the loopback address into its embedded copy either, so the fallback it keeps for a missing web root is correct rather than misleading. Substituted in html, js and mjs, the same set `docker-init.sh` rewrites, and the served size is the substituted one so a response is neither truncated nor left hanging. Nothing exercised the marker the frontend build emits wherever the instance url belongs. Every page in the suite sets `remark_config.host` from its own origin, so the compiled-in fallback is never read, and a distribution that stopped substituting would keep the suite green. Two tests. The first reads the served bundles and pages back and asserts the marker is gone from each and that what replaced it is this instance. The second covers what the substitution is for: the widget document carries no host of its own, since `iframe.html` builds its config from a query string the parent never puts one in, so everything it requests is addressed with the compiled-in url. It asserts the widget renders and that the config request went to this instance. The demo pages cannot show the second. Their loader builds the bundle's own script url from `remark_config.host`, so a page without one never gets as far as loading the widget. Verified by disabling both substitution paths, the serve-time one and the docker image's, and rebuilding: both tests fail. Editing the files on disk is not enough, since the file server substitutes as it serves. The served body now depends on remarkURL, but cacheControl builds its etag from version and path only. An operator who notices the widget is addressed to the wrong host, corrects REMARK_URL and restarts the same binary gets 304 on revalidation, so the client keeps a bundle pointing at the old host. Cache-Control is no-cache, so it revalidates every time and never ages out of that state either. That is the exact situation this substitution exists to fix, so the validator has to carry the url. |
||
|
Dmitry VerkhoturovandGitHub
|
23be25d84a |
Fix seven widget defects, including the cookies the separate-domain setup needs (#2197)
* Drop the frontend workspace root and re-resolve the lockfile
`frontend/` carried a `package.json`, a `pnpm-workspace.yaml` and the lockfile
for a workspace of exactly one package. Two manifests meant two places to
declare a version, and the app pin was the one that did not win: `preact` and
`@babel/core` were each written twice, and a bump to the app manifest alone
would have been a silent no-op, since `pnpm.overrides` decides and it lived at
the root.
Everything pnpm reads now lives in `frontend/apps/remark42`: dependencies,
`packageManager`, `engines` and the overrides. `frontend/` keeps `.nvmrc`,
`.husky` and `CLAUDE.md`, none of which pnpm reads. The directory nesting
stays: every path in the repository points at `frontend/apps/remark42`,
including the published contributing docs, so moving the package up would have
rewritten 14 files to no benefit.
Moving the manifest kept the old resolutions verbatim, which left optional peer
subtrees the tree no longer reaches: `ts-node` under jest, `@swc/core` under
webpack, `vitest` under `@testing-library/jest-dom`, `tslib` under
`webpack-dev-server`. None is referenced by any config or source file here.
Re-resolving drops 137 packages and moves 59 to versions already permitted by
the ranges in the manifest, 1446 to 1308, with no direct dependency changing
version: the five that look changed differ only in their peer suffix. Every
file `pnpm build` produces is identical in size before and after.
The frontend-deps stage of the Dockerfile sets `CI=true` so the `prepare`
script skips husky, which has no git repository to install hooks into there.
* Stop markdown-only changes triggering heavy workflows, and check the documented versions
`ci-backend.yml`, `ci-build.yml` and `ci-frontend.yml` all end their path
filters with `!**.md`. The e2e workflow did not, so a change to any markdown
file under `frontend/` or `backend/` matched its `frontend/**` and `backend/**`
entries and started a docker build and the whole browser suite. The release
filter had the same hole and two of its own: it names `README.md` and `LICENSE`
on purpose, since `.goreleaser.yml` packages both, so it now excludes markdown
under `backend/` and `frontend/` only. `CLAUDE.md` and the installation page
were listed as well, and neither is packaged.
`ci-site.yml` goes on matching markdown, which is right, since the site is
built from it. It excludes `CLAUDE.md`, so a future `site/CLAUDE.md` cannot
start a site build, and `site/README.md`, which documents how to build the site
rather than being part of it.
The installation page tells a reader that a source build needs Go 1.25, Node
24+ and PNPM 10. Nothing kept those in step with `backend/go.mod`,
`engines.node`, `packageManager` and `.nvmrc`, and the drift is silent: a wrong
version in the docs builds and tests exactly as well as a right one. `.nvmrc`
is the pin with form here, having sat at 16 through the whole node 20 migration
because nothing red ever pointed at it. The check compares each stated version
against its source and holds `.nvmrc` to `engines.node`, and it fails when the
page states no version at all, so removing the claims cannot turn it into a
check that passes by comparing nothing.
Its own workflow rather than a step in an existing one, since the inputs span
the backend module, the frontend manifest and the site.
* Fix the cookie fallback page, asset path, message senders, auth teardown and cookies
Two defects with the same origin:
|
||
|
Dmitry VerkhoturovandGitHub
|
4d5dae20e2 |
Broaden the e2e suite from 21 cases to 63, and harden its harness (#2196)
* Pin the published /web surface in the e2e suite
#2178 renamed the widget bundles from .js to .mjs and the URLs earlier
releases served under those names stopped resolving. Three were noticed
from the demo site; the rest, including every locale chunk, were found
only by requesting the whole surface of both images over HTTP. #2192
restored them with a server-side alias, and nothing in the suite would
have caught the break or would notice it returning.
Two cases with deliberately different criteria. The documented names are
written out, because the documentation decides that list and not the
build: an operator pastes privacy.html into an OAuth application, the
nginx manual proxies index.html by name, and the integration guides start
from the embed script. Everything else is taken from the build itself, so
whatever the bundler emitted has to serve identical bytes under its
legacy .js name and parse as a classic script, which is the premise
serving one under the other rests on. A third case requests a name that
does not exist, without which a fallback serving one page for everything
would keep the whole table green.
All of them check the content type as well as the bytes: nosniff is set
on every response, so a bundle served as text/plain is as broken as one
that 404s while comparing equal.
On
|
||
|
UmputunandGitHub
|
a82dc8d3f1 |
Restore the legacy /web/*.js URLs and fix iframe reuse (#2192)
* Serve the legacy /web/*.js names from their .mjs siblings The build emitted <name>.js alongside <name>.mjs until the two compilations were collapsed into one. Dropping the second compilation was right, but it removed URLs the project itself had published: the v1.16.4 SPA documentation named /web/embed.js directly and its loader snippet requested .js. Pages that hard-coded those names now 404 with no deprecation. webFiles.Open retries a missing .js against the .mjs sibling. The bundles contain no import or export, so the same bytes serve both names. The retry runs only once both sources report the name missing, so a real .js still wins, and an unreadable sibling reports its own error rather than being flattened into the requested file's 404. Related to #2178 * Reuse only the comments iframe embed created createInstance took root.firstElementChild as its iframe, so anything a page left inside #remark42 was adopted instead. A <noscript> fallback became the "iframe", createIframe never ran, and the height messages went to an element that cannot show comments. That also defeats the placeholder support, which promises content in the root is cleared once the iframe reports inited: a text placeholder works, but any element placeholder is mistaken for the iframe, so inited never arrives and the cleanup never runs. The iframe now carries data-remark42-iframe and the lookup is scoped to a direct child, so a second createInstance still reuses it while nothing else in the root can be adopted. Related to #1990 * Assert the backup contents rather than the compressed size TestBackup_MakeBackup and TestBackup_Do pinned the gzip output at 52 bytes, which ties them to the exact output of compress/flate. The same input encodes to 57 bytes on go 1.27, so both fail for anyone building on a toolchain newer than the one CI pins. They now read the backup back and compare it against what the exporter wrote, which is what the tests were reaching for and does not move with the compressor. The payload is a shared constant so the two cannot drift. |
||
|
Dmitry VerkhoturovandGitHub
|
e3d1d0e23e |
Create the e2e trace directory before writing a trace (#2194)
`newPageOn` writes a trace into `traces/` when a test fails, and never created that directory. It is gitignored, so a fresh checkout does not have it. Traces were not in fact being dropped: the driver creates the parent of the trace path itself, checked against the version this module pins rather than assumed. The directory is created here anyway because nothing in the suite states or tests that dependency, and the missing directory has been raised in review on #2180 and again on #2193, each time needing the driver checked before it could be answered. One visible difference on a fresh checkout: the directory now arrives at 0750 rather than the 0755 the driver's own mkdir leaves, both measured. It runs only on a test that has already failed, and logs its error rather than swallowing it, matching the Stop call below it. |
||
|
Dmitry VerkhoturovandGitHub
|
49bf83b09c |
Address the review follow-ups from #2188, #2189 and #2190 (#2193)
* Read the collapsed-threads key through getJsonItem `getFromLocalStorage` parsed the stored string directly, so anything malformed under `__remarkCollapsed` threw out of `restoreCollapsedThreads`. That call sits in `remark.tsx` ahead of the `render`, so the throw took the whole widget with it: the reader was left on the preloader, over a view preference. `getJsonItem` in `common/local-storage.ts` already wraps a parse of a localStorage key and returns null on failure, and null is a shape the check below already reads as empty. The rest of that function is total against whatever the browser holds, and the bare parse was the one way in. * Stop retrying a failed e2e test in CI The suite went in with one gotestsum rerun. It has no failures on record to justify that: 31 CI runs since it landed, all green, and no rerun report has ever been produced. A retry is what turns an intermittent regression into a green build, and while the suite is this young its own failures are the evidence worth keeping. `E2E_RUN_ID` stays. It stamps the threads a run works on with the CI run id, so a thread url in a trace or a log names the run it came from. It carries no data across: the stack is disposable, and a local run under the same id gets those urls on an empty database. * Stop two chooseUnusedPort comments claiming collisions cannot happen All four copies listen on :0, read the assigned port, close the listener and bind later, so nothing holds the number across that gap and another binary can take it. The copies in app/cmd and app/rest/api call a collision very unlikely, which is accurate; the ones in app and the example module said binaries never land on the same number, which is not, and a comment ruling out a port collision is what would send the next person chasing one somewhere else. All four now read the same. Closing the window rather than describing it means the server binding :0 itself and reporting the address it got, which is a larger change. |
||
|
Dmitry VerkhoturovandGitHub
|
0b651dddd4 |
Make backend tests wait on conditions instead of durations (#2190)
* Make backend tests wait on conditions instead of durations The backend workflow has a long tail of runs that fail once and pass on a rerun. Every one of them comes down to a test assuming an operation finishes within some duration rather than waiting for the state it needs. Three were reproducible and each was reproduced against the old code before being changed: TestServerAuthHooks minted a token that lived one second and never tested expiry, so a slow runner turned the first POST into a 401; TestServerApp_AnonMode saw "connection refused" because waitForHTTPServerStart returned silently after three seconds and left a later assertion to fail with something unrelated; TestFsStore_Cleanup slept 200ms against a 300ms ttl that Cleanup widens to 400ms with its commit grace, so roughly 100ms of stall collected an image meant to survive. Fixed sleeps before asserting on asynchronous work are replaced with polls on the condition itself, using require.Eventually and require.EventuallyWithT, and require.Never where the assertion is that something did not happen. Polling closures assert on the CollectT they are handed rather than on t, since testify runs them on another goroutine, and polls that issue HTTP requests stay under the rate limit on the routes they poll through. Where a test needs time to have passed, the clock input is pinned instead: staging ages are stamped with os.Chtimes on both sides of the cleanup boundary right before each call, which also makes the 100ms commit grace an exact case rather than something no assertion reaches, and the RSS tests set store.Comment.Timestamp explicitly rather than racing the wall clock into the first 100ms of a second so pubDate matches. chooseUnusedPort takes a port from the kernel's ephemeral range. Picking at random out of a fixed 10000-port window let two package binaries, which go test ./... runs concurrently, land on the same number between the probe closing and the server binding. The start helpers fail naming the port they waited on, and the SSL tests wait on the redirect port as well as the TLS one. Arbitrary budgets that nothing tests are gone: ten HTTP clients with a one-second timeout against bolt-backed import and export, the "should take about 100msec" assertions, and a one-second bound on noticing an already cancelled context. Shutdown stays bounded at ten seconds so a hang is still caught. Two assertions get stronger. TestServerAuthHooks accepted 403 or 401 from a blocked user, an alternative that existed only because the short token could expire mid-test; it is deterministically 403 now. TestAdmin_BlockedList asserted two users blocked while one carried the same 150ms ttl the next step waits to lapse, so the halves raced each other. goleak stops reporting the regexp2 clock goroutine, which chroma pulls in for syntax highlighting and which lives for up to a second after the last match with a timeout; it ends on its own but a binary finishing inside that window was reported as leaking, and this suite now finishes sooner. The ignore for net/http.(*Server).Shutdown goes the other way: it no longer matches anything, with both packages run fifteen times each under CPU oversubscription to confirm. Two gaps the change would otherwise have opened are covered directly rather than left to the side effects that used to cover them. The one-second token was the only thing exercising the authenticator's ClaimsUpd hook on refresh, so TestServerApp_ClaimsUpd now calls the hook itself and checks admin, blocked, email and restricted-name impersonation, including the two pass-through cases. Lifting the open-route limit removed the last incidental exercise of the rate limiter, so TestRateLimiter drives a burst past the allowance and checks the refusals and that the limit is per client. Both run without a wall clock, and both were confirmed to fail when the behaviour they cover is removed. Production code is untouched. The two sleeps outside test code, the 429 backoff in cmd/cleanup.go and the submit poll in store/image/image.go, are left alone: no CI failure implicates them. Test sleeps drop from 67 to 21, all of them either inside a testing/synctest bubble or a poll interval. The suite runs in about 22 seconds instead of 46, mostly because TestPublic_FindCommentsCtrl_ConsistentCount no longer paces a hundred subtests with an 80ms sleep each to stay under the open route limit. The 300s per-package budget now matches across both workflows, the race_test target and the documented command, and CLAUDE.md records the convention. with '#' will be ignored, and an empty message aborts the commit. # # Date: Sat Aug 22 01:12:31 2026 +0100 # # interactive rebase in progress; onto |
||
|
Dmitry VerkhoturovandGitHub
|
b6975af63c |
Fix collapsed threads not restoring, and the clock skew correction (#2188)
* Fix collapsed threads not restoring, and the clock skew correction
Collapse state was kept as a flat list of `siteID_url_commentID` strings
and read back by splitting on `_`. Any underscore in the url, the site id
or the comment id made the pieces impossible to tell apart, so a page
whose url contains one lost its collapsed threads on every reload, and one
page's entries could be read or deleted as another's: `/post` matched
everything stored for `/post_2`, and a site id of `blog` matched `blog_ru`.
No separator fixes that, since every candidate can occur inside the values,
so the ids are now nested under the site and the url instead. Anything
stored in the old shape reads as empty: collapsed threads are a view
preference, and re-expanding them once is not worth a migration.
The e2e suite had been stripping underscores out of its own thread urls to
work around this, which left its collapse test unable to fail on the bug it
covers. That workaround is gone, and the test now fails without this fix.
`serverClientTimeDiff` was written in seconds and added to an epoch in
milliseconds, so the correction it exists to apply was a thousandth of the
real skew. It is now milliseconds, and named for the unit.
A response with no usable `date` used to fall back to a zero timestamp,
which already made the "skew" about twenty days and would have made it
fifty-five years once the units were right. Nothing is stored now unless
the reading is plausible, since `Date.parse` is lenient enough to turn junk
into a date and let an absurd value through the branch that parses.
The score tooltip reports controversy again when there is any. It has been
dead since the vote component was rewritten in
|
||
|
Dmitry VerkhoturovandGitHub
|
4fca268dc6 |
Pin staging ages in TestFsStore_Cleanup instead of sleeping (#2191)
The test slept 200ms, ran Cleanup with a 300ms TTL and then asserted the second and third staged images survived. Cleanup collects anything older than the TTL plus a 100ms commit grace, and the second image was already 300ms old by then, so a runner that stalled ~100ms anywhere in the setup aged it past the line and the assertion failed with "file on staging". Age comes from the file's modification time, so the test now sets it with os.Chtimes on both sides of the boundary immediately before each Cleanup call: the image meant to be collected is backdated an hour, the ones meant to survive are stamped at now. That leaves no window for a stall to age a file into the wrong bucket, and drops 600ms of sleeping. Verified by injecting a stall into the setup: 250ms reproduces the failure on the current code, while the version here survives 2s. |
||
|
Dmitry VerkhoturovandGitHub
|
a0879b2336 |
Measure the iframe reveal budgets from inside the page (#2189)
The three reveal tests timed their budgets from before `page.Goto`, so a slow navigation was spent against a window that belongs to the iframe. In `TestIframe_StaysHiddenUntilTheDocumentReportsInited` that made the test vacuous rather than flaky: on a navigation between 2.5 and 5 seconds the loop bounding the visibility assertion had no budget left, ran zero times, and the test passed having asserted nothing. Reproduced by delaying the demo document by three seconds, where the assertion ran 0 times before and runs 23 after. The timeout test had the mirror of it, with navigation counting toward the lower bound that exists to catch a shortened fallback. An init script now records, in the page, when the widget's iframe element enters the document and when its visibility first flips. `create-iframe.ts` arms its fallback a moment earlier, on the detached element, so these read a shade short and every bound is conservative in the same direction. Both bounds were also wider than the thing they guard. The hidden window now runs almost to the fallback rather than half of it, and the lower bound sits just under it rather than at three quarters, which a fallback shortened to four seconds used to clear. CI reruns a failing test once rather than failing the build on the first flake. A browser suite has a floor no amount of care removes, and one flake failing the build is what stops people trusting the suite. Once rather than twice, because a rerun stops at the first pass and each further attempt only widens the window where a real intermittent regression is absorbed. What needed a rerun is written to a report and uploaded with the traces, which are kept whether or not the job went green: a run that recovered on the rerun is exactly the one whose evidence used to be discarded. |
||
|
Dmitry VerkhoturovandGitHub
|
7c312da199 |
Stop the Telegram paragraph rendering with spaces in Japanese and Chinese (#2187)
`telegram-link.tsx` assembles that paragraph from five separate messages with the anchor and the QR clause in the middle, joining them with a hardcoded space. Japanese and Chinese do not put spaces between words, so the assembled sentence carried them mid-clause: `通过 此链接 或扫描二维码 打开 Telegram，` separated a preposition from its object and an adverbial phrase from its verb. The separator now comes from the locale and is empty for `ja`, `zh` and `zh-tw`. Korean keeps its spaces, because Korean uses them, as do Thai's phrase boundaries. The locale is matched exactly as `loadLocale` matches it. Comparing case insensitively would have been worse than the bug: `remark_config.locale` is forwarded verbatim and `loadLocale` is case sensitive, so a conventional `zh-TW` loads the English catalogue, and a lowercased comparison would then join English words with nothing between them. The test covers that case alongside `ja` and `en`, and fails if either the comparison loosens or the separator stops depending on the locale. Macedonian labelled the replies feed as comments. `subscribeByRSS.replies` carried `Коментари`, the same value as `user.comments`, in a catalogue whose two reply strings are both `Одговори`. That option subscribes to `/rss/reply?user=`, which `UserReplies` documents as comments replied to that user, so the feed is replies. `auth.user-not-found` is removed. It reached every catalogue but could not render: the only dynamic path to it is `messages[invalidReason]`, and `invalidReason` comes from `getTokenInvalidReason`, which returns `expiredToken`, `invalidToken` or null, or from a backend error string, and the backend emits nothing matching. Catalogues go from 181 keys to 180. |
||
|
Dmitry VerkhoturovandGitHub
|
a5b2fe3cfc |
Consolidate the frontend toolchain onto babel, and ship one bundle (#2178)
Four upgrades that were finished but never merged, the compiler collapse
they enable, and the dependency sweep that follows. Direct
devDependencies go from 78 to 60 and dependencies from 10 to 9.
Three were doing the same job: `ts-loader` stripped types in webpack,
`babel-loader` did everything else, and `@swc/jest` repeated both for the
tests with its own copy of the JSX settings. Babel is the one that
survives, because the `data-testid` stripper has no equivalent elsewhere.
`ts-loader` ran `transpileOnly: true`, so it only stripped types, which
`@babel/preset-typescript` does; `fork-ts-checker-webpack-plugin` was
already what type-checks. Jest runs `babel-jest` against the same
`.babelrc.js` the bundle uses, passed as `configFile` because a
file-relative babel config does not reach the `node_modules` packages in
`transformIgnorePatterns`, and `jest.config.mjs` is plain ESM because a
`.ts` config is compiled against `tsconfig.json`, whose
`verbatimModuleSyntax` rejects ESM syntax in a file the package has not
declared as a module.
That removes `ts-loader`, `@swc/jest` and `@swc/core`. The last was
pinned to 1.2.205 from 2022 with no way forward, because newer builds
emit non-configurable exports and break `jest.spyOn` across 13 suites.
Babel compiles a file at a time with no type information, so it cannot
tell a type-only import from a real one and keeps the module. One line,
`import { boundActions } from './connected-comment'`, pulled the whole
redux store into `last-comments.mjs` and doubled it. `verbatimModuleSyntax`
and `@typescript-eslint/consistent-type-imports` mark them properly; the
statement has to be a separate `import type`, since verbatim semantics
keep an inline `import { type X }` and load the module anyway.
The legacy and modern compilations produced the same bytes. Both read the
same browserslist query, `defaults, not IE 11, not samsung 12` resolves to
chrome 109 and up, and nothing in the source needs transforming for that
set, so 28 of the 29 output pairs were byte-identical.
That made the module/nomodule switch worse than redundant: it served the
`.js` file to browsers with no ES module support, and those files carried
`??`, `?.` and class fields, so the fallback handed its own audience a
syntax error. There is now one bundle, always loaded as a module, in the
five templates and in the seven `site/` documents integrators copy from.
A production build emits 29 files rather than 58, in about 3 seconds
rather than 17. Two of those documents did not work at all beforehand:
the SPA snippet could not parse, and the subdomain example had an
unterminated string.
`@babel/core` 8 declares `^22.18 || >=24.11` and `size-limit` 13 declares
`^22.18 || ^24 || >=26`, so 20 was below the floor of two things installed
here; pnpm only warns, which is why every build passed. All seven places
the frontend pins it move together. `site/` is untouched: it builds with
yarn and eleventy and installs neither.
`eslint --print-config` before and after gives 173 active rules on an
application file against 172, and 172 on a spec file and a plain JS file
against 171. What is gone is three `flowtype` rules with no Flow here,
`no-new-object` and `no-new-symbol` whose upstream replacements are on,
`react/forbid-foreign-prop-types` with no propTypes anywhere, and, on TS
only, `no-useless-constructor`, whose typescript-eslint version is on at
error. `@babel/core` is pinned to 8 across the workspace because
`@jest/transform` and `istanbul-lib-instrument` depend on 7 outright; a
second scoped override holds `eslint-config-preact` on 7, since its
`@babel/eslint-parser` loads babel 7 syntax plugins.
`fast-async` rewrote every async function into nodent promise chains,
calls babel's `transform` synchronously, which babel 8 removed, and every
browser in the target list runs async natively. `prefresh` blew its stack
on `createContext` under babel 8 with no newer release to move to, which
compiled `intl.tsx` and `store/context.tsx` into throwing stubs, so
`pnpm dev:app` could not run the widget at all. `core-js` is not injected
now that `useBuiltIns` is gone, `postcss-custom-properties` was reached
directly although nothing declared it and resolved only through pnpm's
private hoist directory, and `cssnano` ran in both postcss chains although
`CssMinimizerPlugin` already uses it.
`pnpm lint`, `pnpm test` and `pnpm build` now work from `frontend/` as
`CLAUDE.md` and the contributing guide have always said they do; the
workspace root defined none of them.
|
||
|
Dmitry VerkhoturovandUmputun
|
7ee3a0da48 |
Tidy the example module for the testify bump
Dependabot updates `backend/` only, so the example module that replaces it with `../../` keeps the old versions as indirect entries and the `test examples` job fails with `go: updates to go.mod needed`. Beyond testify itself this picks up the yaml module move, from `gopkg.in/yaml.v3` to `go.yaml.in/yaml/v3`, and drops two indirect entries nothing needs any more. |
||
 dependabot[bot]andUmputun
|
4aaba0fb61 |
chore(deps): bump github.com/stretchr/testify
Bumps the go-modules-updates group in /backend with 1 update: [github.com/stretchr/testify](https://github.com/stretchr/testify). Updates `github.com/stretchr/testify` from 1.12.0 to 1.12.1 - [Release notes](https://github.com/stretchr/testify/releases) - [Commits](https://github.com/stretchr/testify/compare/v1.12.0...v1.12.1) --- updated-dependencies: - dependency-name: github.com/stretchr/testify dependency-version: 1.12.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-modules-updates ... Signed-off-by: dependabot[bot] <support@github.com> |
||
|
Dmitry VerkhoturovandGitHub
|
fb7b6c2cdd |
Serve the build-independent web assets from the backend (#2181)
* Serve the build-independent web assets from the backend `privacy.html`, `markdown-help.html` and the `400x400.jpeg` it embeds carry no template variable, link no script or stylesheet, and are imported by nothing in the widget. They now live in `backend/app/webassets/assets`, embedded there, and are served under `/web` alongside the frontend build. `/web` reads the frontend build first and falls back to them, which is what lets an operator replace one by dropping a file into `--web-root`. That is what `privacy.html` needs: it describes remark42.com, while the authorization guide tells operators to hand its URL to Google and Facebook as their own application's privacy policy. Only a missing file falls through. An unreadable file in the web root keeps reporting as unreadable rather than being silently replaced by the embedded copy, and a name the filesystem rejects reports as missing rather than as a server error, both matching what `http.Dir` did. The dev server serves the same directory, so the Markdown help link in the comment form resolves on the dev port as well as in production. The two pages are served as they are written. `markdown-help.html` was minified before, and its formatted inline stylesheet is most of its 8.5 kB; that is 2.4 kB more over the wire, behind the hour-long cache header the file server already sets. Drops `copy-webpack-plugin`, which had no other pattern, and the stylelint entries that only ever matched these files. * Make pnpm dev:app start again The dev server has been failing to start on two counts, so the flow the contributing guide documents does not run at all. `webpack-cli` 4 drives `webpack-dev-server` 5 through the argument order of an older major, handing it the compiler where it expects the options object. It rejects that against its schema and exits, complaining about an unknown `_assetEmittingPreviousFiles` property, which is a field of the compiler. `webpack-cli` 7 is the release that declares `webpack-dev-server` 5 as a peer. Past that, `http-proxy-middleware` resolves to 4.1.1, which no longer accepts the two-argument call `webpack-dev-server` makes, so the `/api` and `/auth` proxies throw on startup. It is pulled in by the security override for CVE-2025-32996, the only override in the file with no upper bound: `>=2.0.10` matches every later major. Bounding it to the 2.x line keeps the fix and the API `webpack-dev-server` calls. With both in place `pnpm dev:app` serves the widget and the pages under `/web` on port 9000. |
||
|
dependabot[bot]andUmputun
|
123b9328d9 |
chore(deps): bump alpine in /site in the site-image-updates group
Bumps the site-image-updates group in /site with 1 update: alpine. Updates `alpine` from 3.22 to 3.24 --- updated-dependencies: - dependency-name: alpine dependency-version: '3.24' dependency-type: direct:production update-type: version-update:semver-minor dependency-group: site-image-updates ... Signed-off-by: dependabot[bot] <support@github.com> |
||
|
dependabot[bot]andUmputun
|
2bfad021e3 |
chore(deps): bump github.com/mxschmitt/playwright-go
Bumps the go-modules-updates group in /e2e with 1 update: [github.com/mxschmitt/playwright-go](https://github.com/mxschmitt/playwright-go). Updates `github.com/mxschmitt/playwright-go` from 0.6201.0 to 0.6201.1 - [Release notes](https://github.com/mxschmitt/playwright-go/releases) - [Commits](https://github.com/mxschmitt/playwright-go/compare/v0.6201.0...v0.6201.1) --- updated-dependencies: - dependency-name: github.com/mxschmitt/playwright-go dependency-version: 0.6201.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-modules-updates ... Signed-off-by: dependabot[bot] <support@github.com> |
||
|
Dmitry VerkhoturovandGitHub
|
4c9ef37cf1 |
Move the site from eleventy to hugo (#2179)
* Move the site from eleventy to hugo The site is built by a single static binary. No node, no package manager, no lockfile, and the toolchain it needed is gone: eleventy, tailwind, postcss, markdown-it and its three plugins, date-fns, prism, npm-run-all, cross-env and html-minifier-terser. Hugo covers most of that itself. Chroma replaces prism, goldmark replaces markdown-it, `--minify` replaces html-minifier-terser, and fingerprinted asset URLs replace the cache-busting `version` shortcode that stamped `Date.now()` into every stylesheet link. `assets/styles.css` is hand-written, since tailwind was the only reason left to keep a package manager. The palette and the light and dark values are custom properties at the top of the file; the minified stylesheet is 15 kB against tailwind's 46 kB, and the whole build 1.0 MB against 1.2 MB. It was matched to the old one by comparing computed styles rather than by eye, which is how the heading weights and line heights, the list marker colour, and the home page heading and sign-off were caught: the last of those had been carried by tailwind utilities written into the markup. The `::: note` container becomes a `note` shortcode taking the emoji to show. Its closer needs a blank line after it, because a shortcode is not a block rule the way `markdown-it-container` was, and without one goldmark keeps the callout inside the open paragraph. The `overflow-x` wrapper around tables and the heading anchors are goldmark render hooks. Syntax guessing is off. Chroma detected a systemd unit file as gdscript and a chat transcript as mysql, and colouring a snippet as the wrong language is worse than not colouring it. The two chroma themes are scoped to opposite sides of the theme switch rather than layered, because they do not declare the same properties on the same tokens: github gives Error a background github-dark never overrides, and styles Punctuation where github-dark leaves it alone. Layered, either leaves a light value applying on a dark page. `[frontmatter] lastmod` resolves through git, then front matter, then file modification time. Without that chain `.Lastmod` falls back to `.Date`, which is zero when a page carries no date, and every page reads `Jan 01, 0001`. `enableGitInfo` is off because the image build context is `site/` alone, where hugo fails hard rather than degrading; `HUGO_ENABLEGITINFO=true` gives real per-page commit dates locally. Three fixes fall out of the move rather than being sought: - `/docs/` redirected nowhere. The stub was a markdown file whose permalink was a template expression while `markdownTemplateEngine` was false, so it never rendered and the URL 404'd. It is an alias now - `/docs/contributing/` pointed at `/docs/contributing/development/`, which has never existed. It points at the backend page - the 404 page was built to `/404/` and nothing served it. Hugo writes it to `/404.html` and reproxy is told to use it The mobile documentation menu is a checkbox and label. `visibility: hidden` on the checkbox, which is what the old `invisible` utility set, takes it out of the tab order, and a label is not focusable on its own, so the menu could not be opened from the keyboard at all. The checkbox is clipped rather than hidden, and its label shows a focus ring. Content is unchanged. Every code block on every page is byte-identical to the eleventy output; the only prose difference is that two example values, `mysite.com` and a quoted `https://demo.remark42.com`, are no longer turned into links, goldmark's linkify being narrower than markdown-it's. `backend/README.md` and `frontend/apps/remark42/README.md` are symlinks into the docs tree and follow it to `site/content/`, as does the path `release.yml` watches. `frontend/CLAUDE.md` described the site as a node and yarn project in four places. * Keep the heading anchors markdown-it generated Goldmark strips punctuation markdown-it kept, so 22 headings holding a dot, slash, apostrophe, question mark, bracket or em dash would take a new id and any link into one from outside the repository would stop resolving. Those headings carry their previous id as well, as an empty target emitted ahead of the heading by the render hook, from a map of content path to old anchor in `data/anchor_aliases.json`. The map was built by matching heading text between the two builds rather than by position, so it survives a heading being added or moved. The hook rather than markdown, because goldmark's `{#id}` attribute syntax cannot express these: it accepts dots, apostrophes and em dashes but treats a slash, a question mark, a bracket or a percent sign as heading text, which is 11 of the 22. The ids are stored percent-decoded, since a browser decodes a fragment before matching, so `#children%E2%80%99s-privacy` finds `children’s-privacy`. Verified by navigating to the awkward ones against the built image and measuring where the page settles: each lands 112px down, which is the header offset the target carries. Three pages carried no title, so the docs template rendered an empty `<h1>` above the heading their markdown already had. They take their titles from that heading text, so neither the wording nor its anchor changes, and the template's `<h1>` carries an id. One in-page link pointed at an anchor goldmark no longer generates. The heading render hook emits no permalink anchor. The one it replaced was an empty `<a href>` with `pointer-events: none`, so it could not be clicked, and its only job was a `::before` spacer that `scroll-margin-top` on the heading already does. Being an `<a href>` it stayed in the tab order, so every heading was an unexplained keyboard stop: eight on the installation page alone. Fragment navigation still lands 112px down, clear of the fixed header. * Harden the site image build and its CI The architecture guard could not fire. `${TARGETARCH:-amd64}` defaulted before the `unsupported arch` branch was reachable, so a build without buildkit put an amd64 hugo inside an aarch64 image and ran only because Docker Desktop emulates it. Reproduced with `--build-arg TARGETARCH=`: `/etc/apk/arch` reported aarch64 and `hugo version` linux/amd64. An empty value is an error now. `Dockerfile.dev` had the same defect and no smoke step to catch it, so it would have failed at `compose up`. The hugo tarball is verified against the release's own `checksums.txt`, and the match is asserted present before it is used: piping grep straight into `sha256sum -c` left the guarantee resting on what the checker does with empty input. Busybox exits 1 there, so it did fail closed, but nothing in the line said so. Verified against a checksums file that does not list the tarball: the build stops before the install. Hugo exits 0 on an empty content tree and emits a two-page shell, which would have been copied, pushed and deployed. The build asserts the home page and a docs page exist. `site/**` pull requests were never built. The only building job is gated on `github.ref == 'refs/heads/master'`, so on a pull request every job skipped and rendered in the checks list the same way a pass does, and the image was first built on the run that also deploys it. A `validate` job builds it with `push: false`, needing no secrets so it works on a fork. `.github/dependabot.yml` watched `/site` for npm packages that are gone. That entry is a docker one, which tracks the alpine base. It does not track the hugo pin and cannot: the docker ecosystem reads `FROM` references, and `ARG HUGO_VERSION` is a bare string in a download URL, so that one is a manual bump and `site/README.md` says so. `Dockerfile.dev` carries a `COPY`, so the dev image works without the compose bind mount, and compose runs as the invoking user rather than root, which on linux left root-owned `public/` and `resources/` in the checkout. Recorded in the backlog: `master` has `required_status_checks` off with an empty check list, so the new job surfaces a red X and does not block a merge. That is a settings decision rather than a code fix. |
||
|
Dmitry VerkhoturovandGitHub
|
ff77f41a3a |
Move the e2e suite to Go and playwright-go (#2180)
* Move the e2e suite to Go and playwright-go The seven playwright tests in `frontend/e2e` become twenty in `e2e/`, a separate Go module driving the same browsers through playwright-go. The npm project, its lockfile entries, its prettier config and `Dockerfile.e2e` go with it, leaving `frontend/` a single-member workspace. The suite covers posting with markdown, replying and the nesting that implies, editing inside the deadline and the backend refusing one outside it, deleting, voting with the optimistic score observed mid-flight and rolled back on failure, changing the sort, collapse persistence across a reload, dev, anonymous and email sign-in end to end, the profile iframe, and the two scripts that render into the host page rather than the widget's own frame. The rendering tests run in chromium, firefox and webkit. The rest sign in, sign-in needs the dev oauth2 provider, and reaching that by name from the host is chromium-only, so they run there alone. `compose-e2e-test.yml` runs remark42, a second instance with a short edit window so that path does not need a five-minute test, and mailpit, which catches the email verification message the suite reads back. Everything binds to the loopback interface: the stack holds a known secret and an admin shared id, and `go test` can start it unattended. The tests run on the host rather than in a container. Three settings there exist for the tests rather than for realism. `REMARK_URL` uses a hostname because the dev oauth2 server binds whatever host it reads out of it, and a loopback bind inside a container cannot be published. `UPDATE_LIMIT` is raised because the default of 0.5/sec rejects any test posting twice in a row. The suite also paces its own `/auth/` calls, which are capped at 2/sec by a bare literal in `rest.go` rather than by a setting. Each test gets its own comment thread from a query string on the demo page, so nothing has to reset the database between runs. CI gains a vet and lint job for the module, since the build tag keeps it out of a plain `go test ./...`, and uploads a browser trace for any test that fails. `e2e/README.md` carries the rest: how to run it, what the stack is for, and the widget behaviour the assertions have to work around. * Update golangci-lint to 2.13.1 in the backend workflow The pin sat three minors behind what the linter installs locally, so CI checked the backend with an older set of rules than anyone running it by hand. 2.10.1 also fetches its config schema over the network on every `config verify`, which is a failure mode with no bearing on the code. Both targets are clean on 2.13.1, `backend/app` and the memory_store example. |
||
|
Dmitry VerkhoturovandGitHub
|
1bb002348a |
Complete and correct every translation catalogue (#2177)
* Fix wrong and missing translations across 17 locales
`errors.8` is `ErrReadOnly` (`backend/app/rest/httperrors.go:29`), but 13
catalogues carried a copy of `errors.7`, which is `ErrUserBlocked`. A
reader who simply hit a read-only thread was told they had been blocked,
in Belarusian, Bulgarian, Brazilian Portuguese, German, Finnish, French,
Japanese, Polish, Russian, Turkish, Ukrainian, Vietnamese and Simplified
Chinese. Each now says the page is read-only, in the terminology that
catalogue already uses for its read-only badge.
Czech had an off-by-one: `errors.19`, restricted words, carried the text
of `errors.18`, file not found, and `errors.18` was left in English. So a
comment caught by the word filter reported a missing file. Both rewritten.
Also corrected, all of the same class:
- `de` `vote.downvote` had a leading space
- `mk` had dropped `{shortcut}` from the bold, italic and link tooltips,
losing the keyboard hints
- `fi` `comment.pin` was "Sitoo", which means "it binds", and `comment.unpin`
followed from it; `errors.forbidden` was misspelled "Kieletty."
- `be` `errors.failed-fetch` ended in a stray "r"
- `zh-tw` `authPanel.read-only` wrote 唯獨 for 唯讀
- `ja` `errors.conflict` used 相衝, which is not Japanese usage
- `it` `auth.symbols-restriction` misspelled "numberi" and used "username"
where `auth.username` now says "Nome utente"
Nine Finnish strings, `it` `auth.username` and `zh-tw` `comment.time` were
left in English; `comment.time` is a format string and now matches the
other CJK locales at `{day} {time}`.
No English source string changes, and every runtime placeholder is
preserved. Reviewed by two independent passes, which between them reworded
five of these and found four of the pre-existing defects above.
* Complete every translation catalogue
No locale carries English text any more, and the Telegram authorisation
paragraph now reads as a sentence in all 23 of them.
That paragraph is assembled in `telegram-link.tsx` from five separate
keys with the link and the QR clause in the middle, so a catalogue that
translates each key in isolation produces word salad in any language
whose verb does not sit where English puts it. Japanese rendered as
"テレグラムを開く リンクで または QR コードをスキャン そこで..." and Korean, Persian,
Traditional Chinese and Czech had the same break. Each of those now
splits the sentence at the point its own grammar wants, and every
catalogue was checked by rendering both the wide and the narrow layout,
since the QR clause only appears above 768px.
Ten catalogues also had `auth.telegram-link`, which is the anchor text
and reads "by the link", left as "Telegram bot" from an older source
wording, so the paragraph named the bot twice and never said what the
link was for. Six of the block's keys had never been translated at all
in the 16 locales that ship it, and `auth.telegram-message-1/2/3` were
English in nine. None of these were visible to a check for "value equals
the English string", because none of them equalled it.
Two of the six untranslated keys are word for word the English of their
`subscribeByEmail` siblings, so each catalogue's own existing wording was
reused rather than a second phrasing invented for the same sentence.
Quotation marks in that paragraph now follow each language rather than
the English source: «» for be, ua, ru, fa, fr and ar, „“ for bg, cs, de
and mk, „” for pl and ro, ”” for fi, 「」 for ja and zh-tw.
The button the paragraph tells the reader to press is Telegram's own, and
Telegram ships no interface translation for Japanese, Thai or Vietnamese,
so those three now name it the way Russian and Traditional Chinese
already did, with the Latin label alongside the translated one.
Also swept and fixed: Vietnamese "Bằng đã huỷ đăng kí" for "Bạn", Spanish
"ó" for "o", Thai "คลิ๊ก" for "คลิก", an unclosed quotation mark in Arabic
`commentForm.upload-file-fail`, German alternating between tippen and
klicken for the same action, Czech infinitive "Otevřít" where the rest of
the paragraph is imperative, Macedonian "СО ЛИНК" in caps, French
"Sélectionner" on a button the text calls "Vérifier", a missing space
after a full stop in `ua` `errors.9`, and double spaces in `mk`, `pl` and
`vi`.
Left identical to English on purpose, because the word is the same in
that language: `RSS` and `Telegram` everywhere, "Email" in be, it, pl,
ro, ua and vi, "Site" in bp, fr and ro, and "Conflict." in ro. German and
Turkish do not use bare "Site" and say "Website" and "Web sitesi".
|
||
|
Dmitry VerkhoturovandGitHub
|
fc4e10573c |
Replace react-intl and remove React from the widget (#2176)
Second and final step of #2166. `react-intl` is replaced by `app/common/intl.tsx`, a small i18n binding over Preact context, and with `react-redux` already gone nothing holds the React compatibility alias. React is now absent from the lockfile, the installed tree, the config and the bundles: `react`, `react-dom`, `react-intl`, `@types/react`, `@preact/compat`, `use-sync-external-store` and `intl-messageformat` are all gone, along with the `paths` entries in `tsconfig.json` and three babel-loader excludes. Runtime dependencies go from 15 to 10. `preact/compat` goes too, which matters more than its 3.8 kB. Importing it anywhere installs hooks on preact's shared `options` that remap `onFocus`/`onBlur` to `focusin`/`focusout` for every element and make `@testing-library/preact` rewrite `change` to `input`, the two bugs behind #2166, still live until now. `Button` was wrapped in `forwardRef` with no caller passing one, and `TextareaAutosize` now takes its ref as an ordinary prop. The workaround in `sort-picker.spec.tsx` is gone with them, since `fireEvent.change` reaches a `<select>` again. Gzipped, against master: `remark.mjs` 76.17 kB to 56.47, `last-comments.mjs` 37.97 to 18.26, `deleteme.mjs` 14.51 to 8.42. The limits move with them and keep more relative headroom than master shipped. ### The binding `IntlProvider`, `useIntl`, `createIntl`, `defineMessages`, `FormattedMessage` and `IntlShape`. 32 files change only their import. The export names copy react-intl's deliberately: `formatjs extract` finds messages by recognising `defineMessages`, `FormattedMessage` and `intl.formatMessage` in the AST rather than by import source, so renaming one silently empties the catalogue. `frontend/CLAUDE.md` records that, along with the destructive part: `translation:generate` would then strip the unextracted keys from all 24 catalogues and the check would pass. A message the binding cannot parse falls back to the message in the source: a broken, unhandled or nested tag, a brace that is not a well-formed placeholder, and a placeholder naming a value the caller did not supply. `mk.json` and `th.json` carried broken markup and rendered in English; both are repaired, so a catalogue sweep over every locale can now require well-formed markup with no exceptions listed. `translation:check` gained the validation that would have caught them when they were proposed: a translation's tags have to be well-formed pairs of the names the English string uses, with no attributes, and its placeholders have to be ones the English string provides. Leaving a tag or a placeholder out stays allowed. Run against master's catalogues it reports both. ### enzyme `@types/enzyme` was the last thing pulling `@types/react`, so React could not leave while enzyme stayed. Its three test files move to `@testing-library/preact`, which now has no rival: `@testing-library/preact-hooks` had one import left and its own unmet peer warning. `intersection-observer` was a runtime dependency nothing imported, and the `cheerio` override lost its last dependent with enzyme. Enzyme's `.find(X).prop()` threw unless exactly one node matched, so the converted tests assert node counts explicitly to keep that. ### Verified All 181 message ids formatted across all 24 catalogues through both real react-intl and this binding: 4344 comparisons, no differences. From a wiped `node_modules`: `pnpm install --frozen-lockfile`, `pnpm lint`, `pnpm type-check`, `pnpm test` (392 tests, 42 suites), `pnpm build`, `pnpm size-check`, `pnpm translation-check`. |
||
|
Dmitry VerkhoturovandGitHub
|
a91e322d5c |
Replace react-redux with a preact context binding (#2175)
* Replace react-redux with a preact context binding One of the two packages holding the @preact/compat alias in place, and the contained one: the store is plain redux, and the only react-redux import inside it was a single line re-exporting typed hooks. * Drop the now-unused react-redux types * Subscribe before paint and check once on subscribe Previously, useSelector subscribed to the store inside useEffect, which runs after paint. A dispatch landing between render and that effect was never delivered, since the listener did not exist yet, so the component kept rendering a stale value until some later unrelated dispatch happened to differ from the stale ref. Subscribing in useLayoutEffect narrows the window to before paint, and running the check once immediately on subscribe closes it, which is what react-redux does for the same reason. Adds the first tests for the binding, one of which fails without this change: the store holds 1 while the DOM still shows 0. * Only re-check on subscribe when the state actually moved The subscribe-time check ran unconditionally, so it re-ran the selector at mount. A selector building a fresh object fails Object.is against the value the render already computed, which forced a second render of every connected component: ConnectedRoot and every ConnectedComment, so around 201 extra renders for a 200-comment thread. Reducers return a new root object on every change, so an unchanged state reference means no dispatch was missed and the check has nothing to find. Comparing against the state the render used keeps the property the check exists for while dropping the extra render. The race test still exercises the guarded path, since its dispatch produces a new state object, and a new test pins the mount case: it fails without the guard. Raised by umputun in review. |
||
|
Dmitry VerkhoturovandUmputun
|
931f2db4e3 |
Drop turbo
CI never invoked it, and after #2172 removed the four api scripts its only remaining job was orchestrating one script in one package. |
||
|
Dmitry VerkhoturovandGitHub
|
b8f6dc5f91 |
Require node 20 and record every place the version is pinned (#2168)
* Require node 20 and record every place the version is pinned The declared floor was >=18 while CI, Docker and both .nvmrc files had been on 20 since the pnpm 8 to 10 migration, and transitive dependencies now require 20.18.1. The docs had drifted further still, telling contributors to install Node 16 and PNPM 8. * Set the node floor to the strictest dependency and keep one checklist undici needs >=20.18.1, so a bare >=20 advertised support for 20.0 to 20.18.0, which fail dependency engine checks. frontend/CLAUDE.md already carried a pinning checklist, so the new entries fold into it rather than starting a rival list in the root file. * Keep the node floor at the major, not a patch version engines.node states the major we support. Individual dev dependencies can be stricter within it, and chasing those patch floors into engines and the docs would turn every lockfile refresh into a docs change. |
||
|
Dmitry VerkhoturovandGitHub
|
b03dc366f9 |
Update preact to 10.29.8 (#2163)
* Update preact to 10.29.8 Also moves TypeScript to 5.9, which preact 10.29 typings require, and the compat and testing library pins that go with it. Type checking resolves JSX from preact via the automatic runtime; the bundle keeps the classic transform so babel still strips test ids. * Move babel to the automatic JSX runtime and refresh frontend notes Leaving babel on the classic h pragma while tsconfig used the automatic runtime meant a tsx file without an h import would type-check and lint clean, then throw at runtime, since eslint-config-preact disables react/react-in-jsx-scope and no-undef is off. * Address review findings on the preact upgrade Forward the textarea ref with useImperativeHandle so it clears on unmount and lands during commit rather than after paint. Pair typescript-eslint with the TypeScript it now has to parse. Use the preact namespace types rather than the deprecated JSX aliases, and drop the redundant type re-declarations the element-specific interfaces already provide. * Drive the focus tests through real DOM focus and blur Dispatching a synthetic focusin hard-coded preact/compat's internal alias for onFocus. Calling focus() and blur() exercises the sequence a browser produces and stays correct if that mapping changes. * Raise the two bundle limits the preact upgrade pushes past CI measures remark.mjs at 78024 bytes against a limit size-limit reads as 78000, so it failed by 24. last-comments.mjs had 36 bytes of headroom and would have tripped on the next change. * Regenerate the lockfile after the rebase The rebase resolution left it missing the @typescript-eslint entries, so every CI job failed at pnpm install --frozen-lockfile. |
||
|
Umputun
|
36062de0e7 |
docs: drop the npm deprecation backlog item
remark42 deferred work belongs in the pull request response where paskal and akellbl4 will see it, not in a file. |
||
|
Umputun
|
90766d6637 |
ci: add umputun as a frontend code owner
frontend/* required @akellbl4 or @Mavrin, so umputun could not satisfy the code-owner rule on any frontend pull request. #2172 needed an admin override and #2163 could not use one, because GitHub routes stacked pull requests through the async merge endpoint, which applies no override. |
||
|
Umputun
|
a1dbb2cb92 |
ci: run frontend checks on any frontend change
The path filter matched only frontend/apps/remark42/**, so a change to the workspace root ran nothing: no lint, type-check, tests or size-limit, and no docker build either since docker.yml waits on this workflow by name. #2160 rewrote pnpm-lock.yaml and the override block, and #2172 removed a workspace package and its CI workflow. Neither ran a single frontend check on its PR or on master. |
||
|
Dmitry VerkhoturovandGitHub
|
d370b78613 |
Drop the @remark42/api package (#2172)
* Drop the @remark42/api package It cannot authenticate anyone: clients/auth.ts exposes only anonymous, email and telegram, with no OAuth method, and the fetcher never sets credentials so its cookie auth cannot work cross-origin. Nothing in the repo consumes it, no third-party consumer exists, and npm has served an alpha from July 2022 that CI never publishes. * Drop the removed workflow from the pnpm pinning checklist frontend/CLAUDE.md still counted ci-frontend-api.yml among the places the pnpm version is pinned, and stated a fixed total that no longer holds. |
||
|
Umputun
|
439ccfa83c | docs: note @remark42/api is still published and undeprecated on npm | ||
|
Dmitry VerkhoturovandUmputun
|
29627f4bf0 |
Raise site resolution floors to clear remaining advisories
Both floors are bounded on the upper side, as an open-ended resolution lets yarn cross a major version. |
||
|
Dmitry VerkhoturovandUmputun
|
164eb89c60 |
Raise pnpm override floors to clear all frontend advisories
All 23 open Dependabot alerts against frontend/pnpm-lock.yaml resolve to packages whose override floor sat below the patched release. Every floor now carries an explicit upper bound, as an open-ended floor lets pnpm resolve across a major version. |
||
|
Dmitry VerkhoturovandUmputun
|
09110c792f |
Bump backend Go modules to latest
Updates every backend dependency with a newer release available, and tidies the example module alongside as any change to backend/go.mod requires. |
||
 
|
3f5b3cdd98 |
feat: add configurable SMTP HELO hostname (#2146)
* feat: add configurable SMTP HELO hostname Allow the SMTP HELO/EHLO hostname to be configured separately from the SMTP server hostname. This is useful when the SMTP server requires clients to identify themselves with a fully qualified hostname different from the server address. * chore: remove vendored dependency changes * Bump go-pkgz/notify to v1.4.0 and document SMTP_HELO_HOST The HELOHost field lands in go-pkgz/notify v1.4.0, so the branch needs the bump to compile; v1.3.0 in master has no such field. The example module is tidied alongside, as any change to backend/go.mod requires. Documents the parameter in the parameters table and, separately, in the email setup page: what it does, that leaving it unset keeps the previous `localhost` greeting, and the case it exists for, a relay refusing the greeting under Postfix `reject_non_fqdn_helo_hostname`. Also records the current limit: verification emails for email authentication go through go-pkgz/auth's own sender, which has no equivalent setting, so the greeting there is unchanged. * Bump go-pkgz/auth to v2.2.0 and apply SMTP_HELO_HOST to verification email The verification email sender had no way to set the greeting, so a relay that refuses the HELO would accept notifications and still reject sign-in emails. EmailParams gains HELOHost in go-pkgz/auth v2.2.0, so the same SMTP_HELO_HOST now drives both paths. The example module is tidied alongside, as any change to backend/go.mod requires. --------- Co-authored-by: oli <someone@somewhere.tld> Co-authored-by: Dmitry Verkhoturov <paskal.07@gmail.com> |