- SocketService's prepare-shutdown hook awaited io.close(), which also
closes the http server and waits for every HTTP connection to end. A
connection mid-request when close() runs stays open for the keep-alive
timeout, so shutdown stalled before PuterServer severs connections. The
hook no longer waits on the http close.
- Test servers disable Ollama unless a test configures it. Its boot probe
logged after files had ended, which failed shared-worker shards with
EnvironmentTeardownError.
- vitest ignores that teardown error for a console call still in flight.
- setupPuterTestEnv boots again on a fresh port after EADDRINUSE, and a
failed setupTestServer start shuts the server down.
- UploadReservationStore expiry tests pin the clock instead of racing a
50 ms TTL.
The stall watcher kept only the longest gap, so a saturated loop with many sub-second stalls still failed database-liveness. It now sums blocked time across the probe (ignoring timer jitter) and reports total and longest.
- The per-account overspend alarm now reads in credits and raises at
warning so it reaches the pager instead of only chat.
- A non-stream completion with no choices (an upstream answering 200 with
an error body) threw a TypeError that classified as internal_error and
paged critical. It now throws bad_response, which falls back like any
other empty response and surfaces as a 400.
* fix: let a revocation be seen by the reads and the nodes that outlive it
Four places where a revoke lands on the primary and the next reader can
still be told the old answer.
The u2a cache key was dropped but not re-filled, so the next miss read a
replica that may still carry the row and cached it for the TTL -- and a
revoke's own settling pass is usually that miss. It re-fills from the
primary, as the u2u side already does.
A revoked session dropped its keys the same way. It now leaves the
revoked row in the cache, read from the primary, so there is nothing for
a lagging replica to re-introduce.
A link share is its own authority rather than an index over a
permission, so a replica still carrying a withdrawn row answers with it.
For a short window after one is withdrawn, those reads go to the
primary.
Socket eviction rode a local event, so it only reached the node that did
the revoking. It is announced on the fan-out channel too, which is where
the node actually holding the connection hears it.
* refactor: carry the revocation on outer.* instead of outer.pubsub.*
Eviction runs through `io.in(room).disconnectSockets()`, which goes via the
socket.io Redis adapter a cluster shares, so one node handling the event
disconnects that user on every node in it. Fanning out to siblings had each
of them repeat the same cluster-wide disconnect.
The adapter's Redis is per-cluster, so peer clusters still need the hop --
`outer.*` reaches them and skips the sibling fan-out. The local
`auth.sessions.revoked` goes with it: `outer.*` fires on the emitting node
too, and nothing else subscribed.
* fix: a signed overwrite lands on the live key only once its completion is accepted
A presigned overwrite URL was signed against the live object key, so the
browser's PUT replaced the owner's bytes directly. That PUT never passes
through Puter, which leaves the access check on `completeWrite` with
nothing left to protect: by the time it runs, what /read serves has
already changed.
The upload now goes to a key of its own. `completeWrite` is already the
authorized step, so that is where the staged object is copied onto the
live key, server-side, and the staged one dropped. The entry keeps its
uuid, so everything written against it still holds. Aborting deletes the
staged object, which the old shared key made unsafe to do.
`copyObjectOfAnySize` carries the copy past the single-request ceiling
by copying part by part, so a large file is no different.
Two behaviours follow from the key no longer being shared: completing an
overwrite that uploaded nothing is refused rather than reported as an
overwrite of unchanged bytes, and abort always drops the session's
object. Only the two presigned paths changed; `prepareBatchWrites`
uploads server-side within one authorized request and still writes in
place.
* fix: the review's findings on the staged overwrite
The batch signed path still signed against the live key, so the route
the SDK's signed upload actually takes was unchanged. It stages now,
which also makes the abort simplification safe: the guard that had
protected a shared key was removed while that path still shared one, so
aborting a batch overwrite would have deleted the owner's object.
The completion is allowed against a path but lands on the uuid captured
at start, and a move between the two puts the bytes somewhere the caller
was never authorized for. The promote refuses unless the entry is still
where the check looked.
The batch promote failing left every session pending with its lease
held; it now fails them as the rest of the function does.
The pending GUI tile and the thumbnail key were keyed on the staged
uuid, which no entry ever carries, so neither correlated with what the
completion emitted. Responses carry `entryUid` for that.
* fix: do not treat a pre-staging session's key as this upload's own
A session signed by the previous release points straight at the live key,
since `objectKey` was `existingEntry?.uuid ?? uuidv4()`. Completing one is
already handled -- the promote returns early when the two keys match -- but
abort deleted `objectKey` unconditionally, which during a rolling deploy
would take the owner's file rather than the upload's litter.
Abort now deletes only a key the upload owns. The multipart branch was
always safe: discarding parts never touches an object already in place.
* fix: settle which default plan an org seat resolves to, and what counts as paid
Default subscription resolvers ran in registration order, which is boot
order: whichever layer registered first answered first. A seat is
provisioned by a team and its address is never confirmed, so the seat
resolver now states a priority and wins over anything a deployment
infers from an address.
The seat cap asked whether the owner's plan was outside the free set,
which a default plan can be without anyone having bought it. Metering
now reports whether an explicit subscription answered, and the cap rests
on that.
* fix: the review's findings on plan resolution
`fromDefault` was read off which resolver answered, not off which policy
applied. An explicit id naming a policy nobody registered falls through
to the default one, and that read as bought. It follows the applied
policy now.
A deployment with unlimited metering has no explicit resolver, so every
owner on it became unpaid and its teams dropped from the paid seat cap
to the free one. Unlimited answers for what a paid plan would, so it
counts as one.
Also: the new constants had been placed between `FREE_SUBSCRIPTION_IDS`
and its docblock, the register docstring still described first-wins, and
prettier had reflowed an untouched licence header.
* feat: a seat confirms the address its team typed in
An address on a seat is the team's claim about someone else's inbox, and
nothing asked the inbox. It is now demanded and sent a code, so the
address counts for nothing until the seat hands it back. Resolvers that
read a domain are the reason this matters.
No address is still fine and demands nothing: the team is the trust
anchor for the account, not for a mailbox it does not hold.
`/confirm-email` allows an unconfirmed actor, so a seat can sign in with
its temporary credential and clear the demand; nothing locks it out.
* fix: clear the confirmation demand with the address it names
`clearUnconfirmedEmail` releases a seat's address so the inbox's owner can
sign up with it. The demand to confirm that address has to go at the same
write: the gate reads `requires_email_confirmation && !email_confirmed`, and
once the address is gone there is nothing left to send a code to.
* fix: keep a seat's address correctable, and only ask what can be answered
Review of the confirmation demand turned up two ways a seat could be left
holding one it had no way to meet:
- With no mail transport configured, the code is never sent and a resend
cannot reach the seat either, so the demand stood with nothing able to
lift it. The seat is now gated only when a code can actually travel.
- An address the owner mistyped gated the seat on an inbox nobody could
read, and the gate itself closes `/user-protected/change-email`, the
route the seat would have fixed it by. `updateSeatEmail` lets the owner
who typed the address retarget it, and refuses once it is confirmed,
when it belongs to whoever proved it.
Also resolves the owner's plan sequentially, so the second read takes the
cache the first wrote and the pair cannot come from two resolutions, and
restores the licence header a jsdoc reflow had rewrapped.
* fix: let the confirm-email window open before the profile has loaded
Booted from a stored token, the window renders before `window.user` is
populated, and reading `.email` off null threw inside the window itself --
so the one screen that could have asked for the code never drew, leaving a
blank page. Reproducible without a team: sign up, then load the GUI again
before confirming.
The address is now encoded rather than interpolated raw, and both forms of
the line are i18n keys; it was hardcoded English before.
* fix: keep a file name to one line in a subject, and the inbound total to the account
**A name reaches the digest subject as the user typed it.** Subjects compile
with `noEscape` on purpose -- a header is not HTML, and escaping would show
entities to the reader -- and the comment there says free-form values collapse
whitespace upstream. A file name does not: `#assertUsableName` refuses only an
empty name, a slash, `.` and `..`, so a name may carry newlines. It is
collapsed to one line and bounded before it goes in.
**`includeTotal` answered an app with the account's figure.** The page is
filtered by what the caller can reach -- an app sees only what its user handed
it -- while `countByHolder` counts every inbound share. The docs already call
`total` an approximation for the filtering; they did not cover an app reading
the size of everything else. It is answered only for a caller acting as the
account now, and the page documents that.
The team-share fixture built its actor as `{ user }` with no `effectiveApp`,
which reads as an app rather than a session. A session acts as the account, so
it says so.
* fix: stop `see` reaching inside a folder, and bound mode changes
**A `see`-only folder recipient was pushed every child's name, size and type.**
The fan-out picked holders by reach -- any live grant on a reaching root -- and
`see` is a grant. But `see` says a node is there; `list` is what says what is
inside it, and the payload carries exactly what `list` would: name, path,
`is_dir`, type, size, modified. Descendants now go only to a holder whose mode
reaches inside. A change to the shared node itself still goes to everyone who
holds it, which is what `see` covers.
**Mode changes skipped the daily quota entirely.** Not spending share budget is
right -- changing a mode is not new reach, and the existing test says so -- but
unbounded it lets one account write a pair as fast as it can ask. They have
their own budget now, ten times the share limit by default and configurable,
so the exemption stays an exemption from *that* budget rather than from any.
* fix: let a half-finished downgrade be finished, and stop Trash answering
**A mode downgrade that failed between its two halves could not be repaired.**
`setUserUser` grants the new mode and then revokes the superseded ones. If the
revokes did not all land, the higher grant stood -- and the retry returned
early, because the mode it was asked for was now present. "Nothing to do" now
means this mode is the only one standing, so asking again is what finishes it.
The same two halves, and the same early return, were in the group variant.
**Trashing someone else's item answered for what is in their Trash.** Moving
into the owner's Trash is deliberately authorized without write on it -- that
is what lets a recipient delete from a shared folder -- but a name collision
there came back as `An entry already exists at /owner/Trash/<name>`. The mover
picks the name, so that is a question they can ask about a folder they cannot
list. It dedupes instead, which is also what the GUI wants: trashing should
not fail over a name.
* perf: record the entry's owner on the share row
The outbound listings answer "shares on things I own that someone else issued"
by joining `share` to `fsentries` and filtering on the owner there. Neither side
of that join bounds the work: driven from `fsentries` it walks every file the
user owns, driven from `share` it walks every share until it has a page. For a
user who owns a lot and delegates nothing, that is the whole table for an empty
result. `#outboundAppsSql` is worse than the ticket says -- it has no LIMIT at
all, so every call aggregates the join across both tables.
The owner now rides on the row, indexed with `id`, so all three read it
directly. An entry cannot change owner -- a move out of the owner's tree is
refused -- so it is written once, with the row.
Set in SQL from `fsentry_id` rather than threaded through callers: four insert
sites, each of which already has the id, and none of which can now forget.
The risk in denormalising is a write path that does: a row with no owner
recorded is a share that silently stops appearing in its owner's listing. The
new test covers exactly that -- it fails when one insert is made to write NULL.
Verified against a real boot: a pre-existing share row, on a database that
genuinely predates the column, comes out with the owner its entry has.
* fix: the review's findings on this branch
**The Trash collision still had both halves open.** `overwrite` was checked
before the trash case, so a caller setting it -- it is wire-supplied -- could
still delete whatever it collided with in the owner's Trash, and the 409
survived for anyone who did not. The trash case is decided first now.
Dedupe alone did not close it either: a name that changes only when there is a
collision answers the same question the refusal did, and the mover sees that
name even though the path is masked. Trashing someone else's item renames it
every time, so the result carries no signal. `original_name` already records
the real name, which is what a restore reads. Two tests pinned the old name;
they say what it is now.
**The owner column had no fallback.** Reads went only through it, so a row
written by anything that predates it -- a pod mid-roll -- would never appear in
its owner's listing again. The three reads now also cover `IS NULL` rows
through the old join, indexed on the null and empty once none remain.
**The grant became conditional on a replica read.** `if (!already)` skipped it
on a stale row while the revokes below stayed unconditional, which could leave
the holder with nothing where the old early return left their access alone. It
grants unconditionally again; only the early return changed.
**The re-issue budget covered one path of four.** Team, link and invite
re-issues reserved nothing, so three ways round it were left open. They share
one `#reserveRemodeQuota` now.
Two docblocks had been pushed off the functions they describe, and two comments
still described the join that is gone.
* chore: one-line the comments added here
* test: measure the prefix walk by what it keys on, not by the clock
`checks many long paths without re-reading every prefix of each` gave
the walk 1500ms of wall clock. It measured nothing useful: the shape it
guards against — keying each step on the accumulated prefix text rather
than on the parent id — costs 29ms against the current 8ms on that
input, so both ends sit far under the bound. Under coverage the same
test ran 1511ms and failed, which is the only thing the number could
ever report on.
`PathPrefixIds` now counts the characters it keys on, and the walk is
asserted on that: proportional to the input, and twice the depth costs
twice as much rather than four times. Keying on the accumulated text
fails both. The original case keeps its real assertion, that a request
this size is refused.
* fix: keep the recorded entry owner current, and the trash guard on the retry
A move into someone else's tree re-owns the whole subtree, which the
migration comment claimed could not happen. The shares on it are
re-pointed at the new owner, or they drop out of the listing the column
keys and stay in the old owner's.
The unique-violation retry re-entered the collision resolver without the
trash flag, which is the one call site the guard was not threaded
through.
* auth: per-account 2FA code limit, single-use otp-login tokens, recovery mail limits
- /login/otp and /login/recovery-code share a per-account bucket charged on
wrong codes; an otp-login token completes one sign-in.
- /send-pass-recovery-email adds an IP bucket, a per-account cap and captcha,
and reuses the outstanding recovery token instead of rotating it.
- Reauth tokens carry the rejection reason; /signup re-attaches a temp
account only from an expired session.
* auth: app-issued access tokens older than their app stop authenticating
An access token whose row or parent session predates the app now holding its
uid is refused, the same check app and worker sessions already get.
* teams: signup can take an address a seat never confirmed
Password signup, save-account and a provider-verified OIDC sign-up take an
address a team seat holds unconfirmed; the seat's copy is cleared at the
write. A seat address that was confirmed still blocks.
* dev-center: encode dropped directory child names
* fix(events): bill reached deliveries only, bound dispatch reads, seal cursors
- A broadcast socket copy is billed only when a connection for its holder
exists on this node, anywhere in this region (connection count) or, for a
durable row, in a region presence names. Without peers it bills as before.
- Compiled match filters are held in a bounded LRU with a TTL, and dropped
when another process reports a subscription ended.
- GET /events/fetch reads the continuation from `cursor`, falling back to
`after`.
- Cursors from /events/fetch, /events/subscriptions and /events/kv-handles
are encrypted with sealCursor/openCursor; plain cursors still read.
- A durable subscribe counts again after its insert and removes its own row
when over the cap; the generation bump for that removal is published.
- Subscribe checks access before reading the anchor's owner, and an anchor
gone in between answers with the subject the caller sent.
- Dispatch reads at most the rows per token routing can use, scanning large
token hashes; a removal still settles every row on the removed anchor.
- puter.events.unsubscribe() stops routing after the server ends the
subscription or reports it gone; fetch() accepts `cursor`.
* fix(events): forward kv values only to regions with rows that ask
- A region counts its session rows asking for kv values per token and
announces the token's value variant (`v#<token>`) over the existing
`watch` item on the first such row, withdrawing it with the last. Add,
remove, reap, reanchor and refresh all keep it, reading `includeValue` off
the row as it is dropped.
- A kv write reads the value variants alongside the watch index and attaches
the value only to forwards bound for a region that announced one.
* fix(pagination): seal id-keyed cursors on every list endpoint
Apps, subdomains, workers, team (members, directory, teams, audit, own
audit), shares (inbound, outbound), the user-to-user permission audit and
readdir/descendant listings now hand out cursors sealed with sealCursor and
read them with openCursor, so a cursor no longer carries a row id. Plain
cursors issued before still read.
* test(events): pass socket ids to noteConnect in the billing tests
* fix(events): keep each region's presence item written only by that region
- A region that receives a forward for a pair it holds no socket for
retires its own presence item through the pin-ordered leave path,
gated by a region-shared claim. The sender no longer retires another
region's item, and the forward reply no longer carries noSocket.
- Presence reads skip items for regions outside the peer list instead
of retiring them.
- removeConnection decrements and deletes the count in one script.
- A failed join releases the join pin, and any connect that finds the
pin free writes the join.
- A single delivery treats a socket on any node of the region as local,
using the region-wide connection count, and picks the next remote
candidate by the regions already tried instead of by index.
- Correct the connection-count TTL comment.
* fix(events): count presence by live socket and honor the leave window region-wide
- A region's presence for a pair is a sorted set of socket ids scored by
last renewal under a new key, replacing the integer count. A socket
counts while renewed within the concurrency-slot window; add, renew and
remove each prune lapsed members and report the live count in one
script, so a node that dies stops holding the pair.
- The last socket going marks the pair as leaving region-wide for the
grace window, in the same script; a connect clears it. A forward that
lands on another node inside the window does not retire the item.
- Add the events.presence.write counter, labelled by op (join, leave,
refresh, retire).
* fix: settle the url file-access gate from a grant held on a folder above
The gate asks `/auth/check-permissions` with `app_uid`, which resolved
through the permission scan. `fs:` access inherits down a tree and only
ACL walks that, so a grant on a folder never answered for a file inside
it and the gate asked again, writing another per-file row on each Allow.
The `app_uid` branch now routes `fs:<uid>:<mode>` through
`ACLService.check` as the app-under-user actor, which is the same
authority the operation itself will face. Everything else keeps the
scan, including `puter.perms.holds`, which sends no `app_uid`.
* fix: the review's findings on the consent gate
A permission that is not a string reached `startsWith` and faulted the
whole request, where it used to answer false inside the catch; both
parsers now refuse a non-string, which also covers the other callers
that read a request's list.
The path spelling of a permission took the scan while the uid spelling
took ACL, so one file could answer two ways. Both spellings resolve to
their entry and take the same route.
The checks run together rather than one after another: the caller gives
up after five seconds and prompts, and a deep path is several reads.
* fix: pin the token sender, scope the grant flag, and stop two stragglers
Five of the SDK auth-plumbing items.
**The global `puter.token` handler pinned origin but not source.** Its three
siblings all pin both, and each says why: the GUI origin is shared by every
frame on that domain, so origin alone identifies a domain, not a sender.
`signIn` pins its popup and binds `msg_id`, the consent dialog pins its popup,
and the reauth wait pins the embedder. This one pinned the embedder only when
`env === 'app'`. Since `authenticateWithPuter` now hands off to `signIn`, which
adopts the token itself, the embedder is the only sender this handler is left
serving, so it pins in every environment.
**`is_grant_user_app_permission` was a request-wide flag held across an await.**
It tells the `app-root-dir` rewriter that a row is being written, and while it
is set that rewriter resolves to a real `fs:` permission instead of
`PERMISSION_FOR_NOTHING_IN_PARTICULAR`. Anything evaluating an `app-root-dir:`
permission beside the grant -- the grant route runs one under `Promise.all`
with fs work -- read the same flag. It now lives in a scope derived for that
call, so what the write needs to say about itself is not also said to its
neighbours. `runInDerivedContext` is new: `runWithContext` starts an empty
store, which would drop the `actor` the rewriter reads.
**`fs.stat` and `fs.readdir` deduped on parameters alone**, so two identities
in one realm could share a result. They now key on origin and token as
`os/user.js` does, which documents the reason.
**`signIn`'s relay poll never stopped.** Closing the popup settles the promise;
the loop kept asking `/login/wait` every second for the life of the page.
**`triggerReauth`'s five-minute timeout was never cleared**, so a successful
reauth left a timer to fire into a settled promise.
The concurrency test parks the rewrite and reads the flag while it is held --
my first attempt read before the flag was even set and passed either way.
* fix: scope the fs cache per identity, and retarget two paths that never ran
The remaining SDK auth-plumbing items.
**`puter._cache` outlived the identity that filled it.** `_clearAuthToken`
dropped the token and the stored session but left the cache and `whoami` as
they were, and its keys were raw paths -- `item:/alice/Documents` and a
`~`-relative key alike, identical whoever asked. A GUI user switch without a
reload could be served the previous user's stat or readdir. Keys now carry the
origin and token they were read as, built in one place so the writers, the
readers, the invalidations and the warm-up probes cannot drift apart, and
signing out drops the cache and `whoami` with the token.
**The driver permission prompt could not fire.** Both paths waited for a 200
carrying `{success:false, error:{code:'permission_denied'}}`. The driver route
answers `success: true` or throws, so a refusal arrives as a 403 -- the shape
the retry now reads. Its two tests synthesised the old envelope, so they passed
against a response the API does not send; they use the real one now, and the
prompt goes from never called to called once.
This is a behaviour change: a driver call refused for permissions will now
prompt, which is what the path was written to do. Happy to delete it instead if
the prompt is not wanted.
The half-written copy of the same idea in `utils.js` is gone -- it prompted for
one hard-coded interface and its retry was a `// todo`.
**The verification-gate single-flight was keyed on nothing**, so a phone-gated
request in flight beside an email-gated one took the email answer, replayed,
failed again and surfaced raw without ever asking. Keyed per gate: same gate
still shares one dialog, a different gate gets its own.
* test: ask the cache the same way the code does
The API suites built `item:<path>` keys by hand, so they stopped finding
anything once the real keys carried the identity they were read as. They go
through `fsCacheKey` now, which is the point: a test that spells the key itself
passes until the moment the spelling changes, and then fails for a reason that
has nothing to do with the behaviour it is guarding.
These run against a live server, so a plain `vitest run` does not reach them --
they were green locally and red in CI.
* fix: the review's findings on this branch
Eight, and three of them were regressions this branch introduced.
**Pinning the token sender to `globalThis.parent` in every environment broke
the case it was meant to protect.** On a top-level page `parent` is the page
itself, and the token arrives from a popup, so the handler stopped accepting
anything. The GUI posts `puter.token` to `window.opener` for the file pickers
too, not only for sign-in, and this handler is their only consumer -- a site
calling `showOpenFilePicker()` would have signed the user in, dropped the token
and run the following read with none. It now pins the embedder when framed and
a window this SDK opened when not; `UI.js` already tracked its picker popups
for the same reason, so that set is shared rather than duplicated.
**The GUI still spelled a cache key by hand.** `updateSubdomainsForItems` built
`item:<path>`, which no longer matches what the SDK writes, so its `get` could
not hit and its `set` wrote an orphan nothing reads and nothing evicts -- a
published website's badge quietly stopped appearing on a cached render. It goes
through `fsCacheKey` now, and that is the last hand-spelled key in the tree.
**Retargeting the driver permission prompt to 403 was wrong and is reverted to
the deletion the ticket asked for.** The route refuses with `forbidden`, not
`permission_denied`, so the retarget still missed it; meanwhile a driver that
raises `permission_denied` for its own reasons would have opened a prompt that
had nothing to do with the refusal. And the prompt asks for
`driver:<iface>:<method>` while the route checks `service:<driver>:ii:<iface>`,
so granting it changes nothing. The path cannot work as written.
The rest were pre-existing or latent:
`cacheWhoami_` wrote its answer without checking the token it was issued under
was still current, so a reauth landing mid-flight could repopulate `whoami`
with the previous identity. `_dropIdentityCaches` also left `whoamiCache_`,
which other call sites await.
`setAuthToken` now drops the identity caches when the token actually changes:
keys carry the token and the store is persisted with no TTL, so a rotation that
did not go through `_clearAuthToken` orphaned a whole key space with nothing to
reclaim it.
The verification-gate entry was registered after the body could already have
deleted it -- a synchronous throw stranded a resolved "not verified" in the map
for the life of the page. It registers first and clears on a microtask.
`signIn`'s poll is bounded as well as settle-checked: on a cross-origin
isolated page opened from a gesture, nothing watches the popup, so closing it
never settled the promise and the loop ran on exactly as before.
`runInDerivedContext` says in its docblock that callee writes do not survive;
only the one flag needs the scope, and a future caller should not expect to
hand a value back through it.
* fix: the review's findings on the SDK auth plumbing
**The relay bound stopped asking without saying so.** The deadline left the
`signIn` promise pending and its listener attached. On an isolated page opened
from a gesture the relay is the only thing that can settle it -- nothing watches
the popup there -- so a user who took longer than the deadline was left with a
promise that never resolved, and `authenticateWithPuter` queues every later
implicit-auth call behind it. It rejects now.
**Pinning the token source left `signIn`'s own popup untracked.** Only the
picker popups registered, so on a top-level page the global handler bailed on a
sign-in it had opened itself: `puter.onAuth` never fired, and
`puterAuthState.authGranted` was never set. `openAuthPopup` is the one place
auth popups are opened, so they register there.
**A closed popup is ours too.** Rejecting one that had already closed loses a
message from a popup that posts and then closes itself, which is why `UI.js`
tolerates the same case. Kept, with the set bounded instead.
**The gate key carried the factors**, so one route naming them and another not
opened two dialogs for the same gate. It keys on the gate, as `ctx.done` does.
**`fsCacheKey` left `~` unresolved**, so a writer and an invalidation spelling
the same place differently missed each other -- the drift the one key builder
was meant to end.
The dead `opts.permission` went with the path that used to read it.
Four workerd-runner failures are left: `ai > chat stream adapter supports
cancellation`, `kv > a GUI boot key written by another client reads fresh` and
two cache-warmer ones. All four fail the same way on `origin/main`, and the node
and browser runners pass.
* chore: one-line the comments added here
* chore: reflow the comments the removed retry cause left ragged
The in-flight claim's lease (2x the invoke timeout) is also a future score, so
polling heldForMs > 0 could pass before the failed attempt's hold was written:
the backoff test read 600 instead of 2000, and the 404/hang/429/suspended-owner
tests ended while their attempt was still in flight, letting it land in the
next test. Wait on the subscription's failure counter instead, which only moves
after the hold or gap marker is written.
Durable subscribe, list, fetch and unsubscribe now apply the same scoped
token check as the handler, worker and kv-handle routes:
`isAccessTokenActor(actor) && !isAccountContext(actor)`. An access token
an app issued (effectiveApp = the app) is treated the same as one the
account issued: subscribe answers 403 `events_durable_requires_account`,
list and fetch return an empty page, unsubscribe answers 404.
`rowInActorScope` returns false for these tokens, which covers durable
unsubscribe and ack. The session verbs that also use it already refuse
access tokens at the socket handshake.
Full-access tokens, app tokens and sessions are unchanged.
- signup: a username taken by a concurrent signup between the duplicate
check and the insert now returns the pre-check's 400 instead of
surfacing the unique-key error as a 500. The holder is read from the
primary.
- fs move: when the UPDATE trips the (parent_id, name) unique key after
the collision probe missed the occupant, re-read it from the primary and
resolve it the same way the probe would have (overwrite, dedupe, or the
409 item_with_same_name_exists), then retry the move once.
- server health: a check that hits the per-check timeout reports how long
the event loop was blocked during it, like the database-liveness latency
failure already does.
`cleanEmail()` applied gmail's `dots_dont_matter` rule to icloud.com, me.com
and mac.com. Apple allocates the exact string: `j.smith@icloud.com` and
`jsmith@icloud.com` are two mailboxes with two owners. Collapsing them made one
canonical address stand for both.
What that canonical address feeds:
- `findEmailOwner` falls back to it, so an OIDC sign-in whose verified claim is
the dotted address resolves to the account holding the undotted one, and
`linkProviderToUser` has nothing left to object to -- the provider verified a
real mailbox and the account's address is confirmed.
- The unique index on COALESCE(clean_email, lower(email)) reads it, so the
rightful owner of a dotted Apple address is refused signup because the
undotted account already "owns" it.
- Pending share invites are stored and claimed on it.
Subaddressing is untouched: Apple does fold `+tag`, so that rule stays. Gmail
keeps both rules -- there the fold is correct.
A migration recomputes `clean_email` for those domains on all three engines,
since rows written before this still hold the collapsed value and the index
reads the stored one. Verified the SQL against the function rather than by
inspection: same five inputs through `cleanEmail()` and through sqlite give
byte-identical output. It can only split rows apart, never merge them, so it
cannot collide with the unique index.
Not changed: `findUserByEmail` still matches on the canonical address. The
cross-match it documents is correct where the provider really does fold --
`foo.bar+tag@gmail.com` is the same inbox as `foobar@gmail.com`. Requiring an
exact match would break that for no benefit here; the fix is for the data to
say what the provider does.
The in-flight claim's lease (2x the invoke timeout) is also a future score,
so polling for "held" could read it before the failed attempt's backoff
hold was written. Wait for deferAfterFailure to settle instead.
Local testing against a running instance found the service-side fix was not
enough. Both answers were 403, but they did not read the same:
exists -> permission_denied: fs:/other/rd:read
missing -> Cannot create outside your home directory (forbidden)
The second comes from `#planFsPathCreate` in the controller, which probes the
path itself and only calls the create guard once it knows the path is absent.
Its comment said so outright -- checked "only once the path is confirmed
missing, so an existing shared path outside the caller's home is not a new
rejection" -- which is the oracle stated as an intention.
A foreign path now refuses word for word whichever it is. Responses are
byte-identical apart from the caller's own input.
`rewritePermissionForActor` collapses the create guard's `forbidden` the same
way it already collapsed the 404, for the paths that reach it through the
service rather than the controller.
The existing test pinned `forbidden` here; its subject is that nothing is
created in someone else's tree, which still holds. Alongside it is the
invariant itself: refuse a present and an absent foreign path, mask the path,
compare the whole error. Without the controller check that fails.
**The path oracle was not closed.** The rewrite only remapped the *error*, so
success-vs-403 still answered whether a foreign path exists. Narrower than
reported: every other grant path already gates on `canManagePermission`, and
only `grantUserAppPermission` has no authority check -- deliberately, since
granting your own app something you don't hold is inert. It now refuses a
foreign path the actor cannot see, with the same error a missing one gives.
My first test for this passed without the fix: `makeUserActor` creates no
fsentries, so both paths 404'd and nothing distinguished them. It inserts a
real entry now, and the control fails with "expected null to match 403" --
null being the grant that succeeded, which is the oracle.
**`#validateEmail` still failed open** on the main signup / change-email /
confirm path while its OIDC twin was hardened. Same shape as OIDC now: a gate
that did not run is not a pass.
**The 16-permission cap broke `puter.perms.check()`** for longer lists. The
SDK's own docblock promised "one round trip however many are asked about",
which the cap made untrue, so it batches instead. The reported second half --
the UI grant poller burning its timeout -- is not reachable:
`MAX_REQUESTED_PERMISSIONS` already caps that path at the same 16.
**`listener_failed` was set by wildcard listeners.** An unrelated observer on
`email.*` throwing would refuse every signup. Only an exact-key listener
implements the event, so only that one speaks for the gate.
**`whoami()` labelled everything `Unauthorized`.** A 502 with an HTML body is
not a 401; the server's `code` now rides along and the fallback names the
status.
Measured first, against a real server, counting DB round trips: a 40-seat
delete was 284 queries, 7.1 per seat, scaling linearly. Four of those seven
answered for the whole page and had no reason to be asked per account.
120x SELECT * FROM `user` WHERE `id` = ?
41x INSERT INTO `audit_team_membership`
40x SELECT COALESCE(SUM(size), 0) FROM fsentries
40x UPDATE `user` SET `suspended` = ?
40x SELECT `uuid` FROM `sessions` WHERE `user_id` = ?
The page now reads who is already suspended, what each holds, and the rows the
notices need in one query each, and writes the audit as one statement. What is
genuinely per account -- the suspension, its billing event, the mail -- stays
per account under the same concurrency bound.
Same 40-seat delete is now 170 queries, 4.3 per seat.
Three new store methods rather than raw SQL at the call site:
`getSuspendedByIds` reads the primary, deliberately not `getByIds`, which is
cache-first. A stale `suspended` is exactly what re-charges a seat on a retry,
which is the thing this path was just fixed to stop doing.
`getHeldBytesForUsers` is `getHeldBytes` with a GROUP BY.
`appendAuditMany` is `appendAudit` as one multi-row insert, still written
before any suspension, so a failed append cannot leave an unlogged one.
What is left is concentrated in `#suspend`: the UPDATE, the forced re-read the
user store does afterwards to refresh its cache, and the session drop. Those
are shared store paths and batching them means owning their cache invalidation,
so they are not in scope here.
Four, all real.
The gate threw on a non-string. `validator.isEmail` asserts its argument, and
two call sites hand it an unvalidated body value -- `send-pass-recovery-email`
is public, so `{"email": 1}` was a 500 rather than a 400. The check belongs in
the gate, not in each caller, which is the whole premise of having one.
OIDC signup never shape-checked the claim at all. `claims.email` went from a
truthiness check straight into `user.create`, so a provider we do not run could
store exactly the string this branch exists to keep out. `email_verified`
attests the mailbox, not the characters.
The change-email confirm handler promoted the staged column without re-reading
it through anything. A row staged before this deploys still lands on `email`;
one check at promotion closes that window.
The invite path kept its own regex, and it was never the same rule. It admitted
`a..b@x.com`, `.a@x.com`, `a@-x.com`, `a;b@x.com` and `a,b@x.com` -- the last of
which nodemailer reads as two recipients -- each one a permanent pending row
that spent daily quota, which is the failure the comment above the constant
said it existed to prevent. It uses the gate now, and the second regex is gone.
A regression test each, all four confirmed to fail without their fix.
`validator.isEmail` accepts a quoted local part, so an address whose local part
is an arbitrary string passes it. Five call sites relied on that check alone
before writing the address to `user.email`, and the invite path's own shape
regex was equally permissive.
`isStorableEmail` is now the single gate: the library check, plus a refusal of
`<`, `>` and `"`, which are legal in an address only inside the quoted form.
Nothing deliverable uses that form, so the rule costs no real address --
apostrophes and ampersands in a local part still pass, and the suite covers
them.
One function rather than a check each caller has to remember, because the next
place that stores an address will not remember.
Six findings, all real.
`total` described rows the page withheld. The count ran over every live team the
user belongs to, while the app-actor filter ran afterwards on the items only, so
`GET /teams?includeTotal=true` from an app returned `items: []` with a non-zero
`total` -- the exact thing the filter exists to prevent, and newly reachable
because `includeTotal` was inert before this branch. The directory-open
restriction moves into the query, so items, cursor and total all describe the
same set.
The GUI told owners the opposite of what the server now does: "we email the
username and temporary password to it." An owner who believed it would dismiss
the one copy of the password and strand the seat. Rewritten.
`runWithConcurrencyLimit` rejects on the first error but cannot cancel the
workers beside it, so a failed seat let up to seven others finish suspending and
emailing "your team has been closed" for a team that was still live. The page
now settles before the rejection is rethrown, which leaves a state the retry can
resume from instead of an arbitrary one. My earlier claim that failure
semantics were unchanged was wrong.
`#disableSeatForDelete` returned `true` on both paths, so the boolean read as if
it distinguished them. It returns nothing; the counter uses the settled results.
`listTeamsForUser` had picked up a second stacked docblock -- the same defect
this branch removes from `#cached` one file over.
`reissueCredential` sent the provisioning mail, which after the credential
change was a byte-identical duplicate saying nothing about the password it had
just invalidated, while the GUI promises the member hears about it. It has its
own template now.
Regression tests for the count leak and the reissue notice; both confirmed to
fail without their fix.
Both docblocks on the directory route said it carries nothing a colleague
cannot already read through `/members`. It carries `uuid`, and `/members`
withholds `uuid` from everyone but the owner, with its own comment pointing
apps at the directory to get one.
The disclosure itself is the opt-in working as designed: `directory_enabled` is
the owner's consent to expose a roster, every member reaches the route the same
way whether they call it from a session or through an app, and the SDK and the
teams recipes already document the directory as where a stable id comes from.
So the comments are what was wrong, not the route.
Removing `uuid` from the directory would be the other way to make them agree,
but it breaks `listDirectory()` for every caller and contradicts the published
recipes; that wants a decision, not a cleanup commit.
`#resolveRecipient` already refuses to treat an unconfirmed address as an
identity, and says why: "resolving it would hand the share to whoever registered
the address first." `#invite` then looked the same address up again and honoured
whatever that row said about blocking -- `blocksAllShares`, or a block on this
particular sender.
So registering an address you do not own, and never confirming it, was enough to
refuse every invite anyone sends to that mailbox. Nothing about that row is
evidence the mailbox is yours, which is the premise resolution already works
from. A team seat reaches this cheaply: those accounts are provisioned with
`requires_email_confirmation: false` and an address the owner typed.
The block is now honoured only for a row that confirmed the address. The alias
reasoning behind the lookup is unchanged -- a confirmed holder still cannot be
reached through a variant they did not type.
Not addressed here: the same unconfirmed seat still holds the address against
the real owner's signup, because `idx_user_owned_email` keys on having a
password rather than on confirmation. Narrowing that is a schema change and
wants its own ticket.
Provisioning a seat lets the owner supply any `email` that passes a format
check. Puter then mailed a working temporary password to it. Nothing confirms
the owner controls that address -- provisioned accounts are created with
`requires_email_confirmation: false`, because the team is the trust anchor for
the account, not for the mailbox. `resendActivation` reissues the credential and
mails it again, up to the 20/day `team:reset-password` budget, so the same
address can be hit repeatedly.
The credential now stays out of the mail on both paths, which is what
`resetMemberPassword` in the same file already does. Nothing is lost: all three
routes return `temporary_password` to the owner, and the SDK has always
documented that as the delivery -- "The returned password is shown once and is
not retrievable afterwards -- deliver it out of band." The mail still goes out,
naming the account and saying the team will supply the password.
The two tests that pinned the old shape are inverted rather than deleted; one of
them carried the rationale "without it this is the only copy", which the return
value contradicts.
`deleteTeam` tore seats down one at a time, each one awaiting a mail send, so a
full 40-seat team was 40 round trips end to end inside a single request. The
per-seat work is independent, so it now runs under the same bounded-concurrency
helper the rest of the backend uses for fan-out sends.
The loop also had no equivalent of the guard `disableMember` carries. That one
checks `suspended` before emitting, with the reason written down: a second
`team.account.disabled` opens a second byte charge that only one `enabled` will
ever close. Deleting a team emitted unconditionally, so any partial failure
followed by a retry double-charged every seat the first attempt had already
disabled. The seat teardown moves into one method both paths can state the same
way, and a retry now resumes instead of re-charging.
Failure semantics are unchanged: a seat that throws still aborts the delete and
leaves the team live, which is what makes the retry meaningful.
Touching this file reformats a few untouched spans -- it carried ten
pre-existing `prettier/prettier` errors that the repo's own pre-commit `eslint
--fix` resolves on any commit that stages it.
The collapse caught every `subject_does_not_exist` the rewriters raise, not just
the path resolver's. `app-root-dir:` 404s when the caller's own app has no
resolvable root directory, and that answer is theirs to have — four of its tests
said so.
Only a permission that parses as an `fs:` path can oracle someone else's tree,
so only that one collapses now.
The signup paths read `allow` off a payload they initialise to `true`, so a
harness listener that throws leaves the verdict untouched and the signup goes
through. The outer try/catch the report points at is not where it happens:
`emitAndWait` already swallows each listener's error and carries on, so that
catch almost never fires and the throw never reaches it.
`emitAndWait` now records the failure on the metadata it was handed, and both
signup paths — OIDC and the ordinary one — refuse when it is set. Other emitters
are unaffected: the flag is only read where somebody is asking for a verdict.
The report named OIDC alone; the ordinary signup had the same hole, and it is
the path that matters more.
Test boots a server, registers a listener that throws, and posts a signup: 200
before, 403 now.
Two unrelated bugs in the same area.
`stat({ returnSize: true })` summed `fsentries` filtered on the *acting* user,
so a recipient asking for the size of a folder shared with them matched no rows
and read 0. The sum is over the owner's rows; the ACL check that let them stat
it is what decides whether they may ask at all.
`fs-access-levels` destructured the mode out of the permission but kept the
`.slice(1)` that used to skip it, so widening `fs:<id>:see:thumbnail` produced
`fs:<id>:read` and dropped the sub-scope. Anything granted below the mode stopped
being implied by a wider one. Latent — nothing ships a sub-scope today — and the
test fails without the fix.
A share recipient addresses an entry as /<owner>/<uuid>/<name>, which is the
point of the mask: it says where the item is without naming the folders above
it. The 404 raised when a file's backing object has gone put `entry.path` in the
error fields, so reading or copying a shared file whose object had vanished
handed back the real path.
The fields carry the uuid alone now, on both the read and the copy site. The
caller already has it — it is what they named the entry by.
The existing ghost test asserted the path was present, so it is the regression
pin; the copy one was not checking fields at all and now does.
`fs:/path:mode` resolves through `getEntryByPath` with no ACL check, and a
missing entry answers 404 while an existing one carries on to the authorization
check and answers 403. The pair is a membership test for any path on the
deployment: grant routes take the permission as given, so anyone could ask
whether /someone-else/taxes.pdf is there.
The resolver keeps its 404 for a path inside the caller's own home, where
knowing it is missing is the useful answer and reveals nothing. Everywhere else
it answers with the same `permission_denied` the authorization check gives, so
the two cases are one case. ACLService already collapses them; this is the same
rule reaching the grant and revoke paths, which take an actor and so can tell
whose tree is being named.
Internal resolution still goes through `rewritePermission`: only the paths that
take a caller-supplied permission moved over.
The test grants on an existing foreign path and a missing one and asserts the
outcomes are identical; without the fix they are 403 and 404.
The generic test covers the defect's shape; this one is the case the report
describes. `fs-access-levels` puts `manage:fs:<id>` in the higher set for
`fs:<id>:read`, so a delegate holding the manage terminates the issuer scan with
it as the leaf — which is how the old warm came to write a manage permission
under a holder granted nothing but read.
Without the fix the holder ends up with `manage:fs:<id>` in the flat view and
`canManagePermission` answers true for a read-only share, so they can re-share
the entry at any mode.
Daniel Salazar