mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 06:19:49 +00:00
ca158c22d8dfe942e327dc2accc163cbf6dcc7fb
2269 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 xarmian
|
ca158c22d8 |
feat(apps): app install staging: apps/1 manifest, fetch policy, pending reservations, preview (TASK-3397, U8a) (#1770)
SPEC-6 U8a (DOC-3371 §1, §2 steps 1-5, §9). Strict apps/1 parsing; an https/no-redirect fetch policy screened at dial time with an admin private-origin list; pending-install reservations under the owner lock and the instance advisory lock, holding the full charge until the fetch ends; the owner preview with every artifact normalized by the importer's own code (normalizeArtifact, prepareCreateFields, both extracted behaviour-preserving and pinned by a parity golden); apps_enabled and the admin apps settings; a background sweep of expired pending installs; migration 118/092. The preview is advisory: it may refuse what the import would accept, never the reverse; U8b re-checks atomically. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
e5262160e1 |
feat(oauth): install clients, service tokens and the issuance barrier for installed apps (TASK-3394, SPEC-6 U5a) (#1769)
* feat(store): install clients, the issuance barrier and app_token_bindings (TASK-3394) * feat(oauth): install-client service tokens, in-process app-token introspection, public introspection hardening, A0 token gate (TASK-3394) * test(store): the issuance barrier serializes with disable, rotate and re-enable on Postgres (TASK-3394) * test(server): pin each install-token refusal by its own hint; bearer introspection of another token (TASK-3394) * fix(oauth): a grant carries the epoch read before client auth; the install-caller check reads credentials as fosite does (TASK-3394, codex r1) * fix(oauth): classify the token request from the body fosite parses, multipart included (TASK-3394, codex r2) * test(server): account for the install-client token rules' form readers in the body-reader census (TASK-3394) * chore(store): renumber the install-client migrations to 117 / pg 091 after #1768 (TASK-3394) |
||
|
xarmian
|
526001c199 |
feat(appstore): app attachment upload, read rule and caps behind FencedTx (TASK-3396, U7a) (#1768)
SPEC-6 U7a (DOC-3371 §4 Attachments, day-85 revisions): the store, blob and appstore layers of app attachments. Constant-work blob writes (FSStore.Stage/Commit/Abort), the upload hash guard moved into attachments.InFlight and shared with the app store, FencedTx upload with exact install caps recounted under the workspace seq lock plus an in-process admission reservation, the app read rule, variant inheritance of uploader and via_app, the AppAttachment DTO, and migration 116/090 (attachments.via_app). The HTTP route lands with U6 or a U7b on top. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
35e4843772 |
feat(appstore): app comment create, edit and delete inside FencedTx (TASK-3391) (#1767)
SPEC-6 U2b (DOC-3371 §4). comments.source accepts 'app' (SQLite 115 rebuild with rowids, indexes, FTS and NUL triggers preserved; PG 089 constraint swap). FencedTx gains CreateComment, UpdateComment and DeleteComment under the workspace seq lock, with item binding before authorship, author-only edits, an item-bound attachment reference check that writes nothing, and a cross-item-thread refusal on delete. appstore exposes them. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
b572f0739c |
feat(auth): the bot principal for installed apps: users.kind, kind gates on every sign-in door and credential, people-only counts (TASK-3392) (#1766)
* feat(store): the bot principal: users.kind, CreateAppUserTx, kind gates on every credential mint and claim, people-only counts, install-gated app projection (TASK-3392) * test(store): record users.kind in the NUL census baseline (store-written, two values) (TASK-3392) * feat(server): kind gates at the sign-in doors, admin 404 and member 409 for a bot, apps array on the member list, reserved domain refused (TASK-3392) * docs+web: the members apps array (TS type, CLAUDE.md); people-only admin member count test (TASK-3392) * test(server): the U3 owner-webhook strip test plants an install (TASK-3392) * test(server): forgot-password and resend refuse a bot before attempting a mint (TASK-3392) * test(server): a usable base URL so the resend door would really mint (TASK-3392) * test(server): the session doors' own kind checks, with the bot already resolved (TASK-3392) * fix: stored reset/verification tokens, PAT rotation, grants and OAuth bearers refuse a bot (TASK-3392, codex r1) * docs: SPEC-6 §11 Q2 is decided (separate Apps section, no seat); the switches stay pinned (TASK-3392) * docs(server): name the two layers in front of CLI approve's kind check (TASK-3392) * test(appstore): the projection's install gate joins the reviewed door helpers (TASK-3392, rebased on #1765) |
||
|
xarmian
|
a17e2c415e |
feat(appstore): app item create and field update inside the fence (TASK-3390) (#1765)
* feat(appstore): app item create and field update inside the fence (TASK-3390)
SPEC-6 U2a (DOC-3371 v12 §4).
store.FencedTx write methods:
- CreateItem:
- the companion check, then the human create's workspace lock
- the item cap under that lock, only when Options.PlanLimit is set (Pad
Cloud), as the human create does; it refuses with ErrAppItemLimit and no
count
- a random-suffix slug, never a collision scan (a scan would reveal hidden
slugs)
- the row (source app, created_via_app and via_app = the install)
- the initial version (via_app), status transition, item.created event and
decision-job enqueue
- wiki links confined to companion targets
- an in-fence "created" activity
- UpdateItemFields:
- the seq lock FIRST, then the companion and created-by-this-install
checks, then the etag (HMAC over install, item and seq under a
server-only key, compared in constant time)
- a field merge, the status transition, update events and enqueue
- an in-fence debounced "updated" activity
- no content, title, relation, parent or attachment change is reachable
Activities are written on the fence's transaction (seq lock, then the
debounce lock, the order BUG-2777 permits). A wiki link to anything but a
live companion item stays broken.
appstore.CreateItem / UpdateItem validate against the companion schema:
- refused: undeclared, relation, computed, unique-scoped and reserved keys
- coerced and validated as the human path does, with defaults applied
- ANY pad-attachment: token is refused in any spelling, and
[[workspace::REF]] is refused
- the create retries ONLY on the items slug conflict (IsItemSlugConflict),
never on another unique violation
Writer identity: via_app joins the activity debounce key and the version
throttle comparison. The U3 projection block carries creator.via_app.
Migrations SQLite 113 / PG 087: via_app on items, comments, activities,
item_versions and item_links; created_via_app on items. FK to app_installs,
whose rows are never deleted.
Boundary checker: reviewed door helpers (store methods and package-level
functions), each with what it writes, audited by reading every body; only
createActivityQ, enqueueDecisionJobsTx, replaceWikiLinks and writeOutboxTx
write, all on the transaction they are handed. Named function-value entries
(the decision-set resolver, two in-body closures) are keyed by function; a
same-named closure elsewhere is still refused (fixture). stampAttachmentRefsTx
stays unreachable.
Tests:
- write census per mutation on both dialects
- every refusal captures nothing
- the item cap and its self-host exemption
- companion-only links
- interleaved app and human creates never share item_number or seq (red on
Postgres without the lock)
- via_app identity in the debounce and the version throttle
- nine mutants, each fails its test
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* fix(appstore): scoped link resolution, defaults under the rules, a required etag key (TASK-3390)
Codex round 1:
- P1: an app link was resolved over the whole workspace and confined
afterwards, so a hidden item sharing a companion's title could win the
match and leave the link broken: an existence signal. Resolution is now
scoped (linkScope) to companion collections in every query, the
number-only and collection-qualified fallbacks included, so a hidden item
cannot influence the outcome. The human paths pass the zero scope and are
unchanged.
- P2: schema defaults are applied after the key checks, so a default could
land a relation, computed or uniqueness-scoped value, or a pad-attachment:
token. Validation now runs against a schema with those defaults removed,
and the effective fields are re-checked for tokens.
- P2: an empty or short etag key made etags forgeable offline. ETag and
UpdateItem now refuse with ErrNoETagKey unless the key is at least 32
bytes.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
|
||
|
xarmian
|
6066d42115 |
feat(outbox): freeze an app-projection block into item and comment events (TASK-3389) (#1763)
* feat(outbox): freeze an app-projection block into item and comment events (TASK-3389) SPEC-6 U3. A connected app's webhook payload must describe an event as it was when it happened, built from nothing but what the mutation froze. Every single-subject item and comment event now carries an `app_projection` block, computed on the mutation's own transaction: - items: creator (account id, display name, kind), collection_id, and the field values projected by the event-time schema (declared keys only, never relation or multi_relation); - comments: creator, collection_id, item_id, and a parent_comment_id kept only when the parent is on the same item; - comment.deleted computes its block before reapTombstonesTx can remove the parent, so the parent check sees it. The comment payload is built by marshaling the comment and adding the block as a key: models.Comment has its own MarshalJSON, which an embedding wrapper would promote, silently dropping the block. Account deletion scrubs the block by path: creator.user_id and the display beside it. Projected field values are the item's own data and are not walked. The block is for app delivery only. The outbox drain strips every app_projection key at deliverOutboxUnit, the one place a payload reaches an owner webhook (singles and folded bulk batches alike). The block is written for every event for now; the one-installed-app gate lands with the installs table (DOC-3371, U4/U5). * fix(store): a data-shape problem degrades the app projection to partial, never fails the write (TASK-3389) * fix(store): a fields blob with trailing data projects partial; cap the partial-warning registry (TASK-3389, codex r3) |
||
|
xarmian
|
85c49cf2fd |
feat(appstore): FencedTx, the appstore skeleton, and the boundary and write-capture guards (TASK-3388) (#1764)
* feat(appstore): FencedTx, the appstore skeleton, and the boundary and write-capture guards (TASK-3388)
SPEC-6 U1 (DOC-3371 v12 §4): the door every app write will go through, and
the two tests that guard it. No app mutations or routes yet; U2 adds them
under these guards.
- store.FencedTx:
- BeginFenced's first statement reads app_installs (auth_epoch, state,
workspace), FOR SHARE on Postgres, and refuses unless the admitted epoch
is current, the install is active and the workspace matches.
- There is no raw Exec and no free-form query. The lock primitives
(LockWorkspaceSeq, NextItemNumber, NextSeq) take the human item path's
own workspace lock.
- Companion guards: requireCompanionCollection, and requireCompanionItem
for derived rows.
- Every field is unexported, and a zero value refuses every method.
- internal/appstore: holds *store.Store concretely; withFence runs one
mutation in one FencedTx; CheckFence is the live path until U2.
- The boundary test (go/packages + go/types) walks every module function
the package can reach. It fails on:
- a *store.Store method off a reviewed allow-list (stronger than the
spec's deny-list)
- database/sql calls outside the door
- Store.db or (*Store).DB
- an interface call or declaration an internal/store type answers
- reflect or unsafe
- the ordinary attachments Put
One fixture per rule under testdata is asserted to be reported.
- The reverse test: no package outside internal/store and internal/appstore
references BeginFenced.
- storetest.CaptureWrites:
- SQLite: modernc's pre-update hook on a single connection, with a
sentinel write proving the hook is still on it.
- Postgres: statement-level audit triggers installed by the harness.
The harness is proven red on a human create (items plus FTS5 shadow
tables) and on a test-only trigger write at depth > 0, and silent on a
read. The blind inventory is asserted exactly: items_fts, comments_fts,
documents_fts and sqlite_sequence.
- guardConn.Unwrap exposes the driver connection to the harness.
- Migration: app_installs (SQLite 112 / PG 086); its columns are added to
the NUL baseline, since no writer exists before U8, which must validate
origin.
Moved to U2, where they are first used: DebounceActivity, CheckItemCap and
the via_app columns.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* fix(appstore): close the boundary checker's gaps and the harness's coverage holes (TASK-3388)
Codex round 1. Each gap now has a testdata fixture asserted to be reported,
and the six P1 fixtures were checked against the round-0 checker:
- package-var: package-level var initializers and init functions are walked
for the root and every module package it brings in that the human store's
own closure does not
- generic: instantiated generic methods are normalized through Origin()
- embedded-sql: rules check a method's DECLARING receiver, so a method
promoted through an embedded *sql.DB or *store.Store is caught at the call
- func-field: a call through a function value is refused (dynamic-call)
unless it is a parameter of an unexported root function that every call
site passes a literal or a named function
- linkname: a go:linkname directive or an unsafe import in a walked package
is refused
- whole module: the walk covers the whole module, not only internal/
- embedded-store: the old checker caught this only by walking into
DeleteItem's body; it is now caught at the call
Against the round-0 checker, all six P1 fixtures fail (pkgvar, helperinit,
generic, embeddedsql, funcfield, linkname); the extra embeddedstore fixture
passed there, for the reason above.
Harness:
- SQLite: PRAGMA data_version, read on the hooked connection at the start
and end, refuses a capture during which another connection committed. A
replaced connection is refused at the sentinel. Both branches are tested;
the data_version test passes silently without the check.
- Postgres: triggers are ensured at every capture, so tables created later
are covered; a table created during a capture is refused; captures are
serialized per database.
- The Postgres lock tests wait until pg_stat_activity shows a backend
waiting on a Lock, instead of treating 500ms as proof of blocking.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* fix(appstore): refuse every function value instead of verifying callbacks; working-trigger check (TASK-3388)
Codex round 2 found five P1s in the checker and one P2 in the harness.
Four of the P1s were bypasses of one mechanism: verifying where a callback
parameter came from. They were reassignment, aliasing through a pointer, a
method expression shifting the argument index, and interface dispatch past
the call-site census. This is a guard, not a proof, so the mechanism is
removed rather than patched again:
- withFence(fn) is gone; app mutations open and commit their FencedTx
directly.
- Every call through a function value is refused (dynamic-call).
- Any use of a variable, parameter or field of function type is refused
(func-value). This also closes the fifth P1: an exported function-typed
package var in the store's own import closure, run through
sync.Once.Do, whose initializer the walk excludes.
Interface checks now judge the INSTANTIATED receiver, since Mutator[string]
can be implemented where Mutator[T] is not; Origin() is used only for
indexing.
Harness P2: a pad_write_audit_t trigger counts only when it is enabled,
statement-level, AFTER, on INSERT/UPDATE/DELETE, and calls
pad_write_audit_fn. Otherwise it is dropped and recreated.
Fixtures genericiface, callbackparam, methodexprcallback and oncevar each
pass against the round-1 checker and are caught now. A Postgres test
disables the trigger and expects the next capture to see the write.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* fix(appstore): refuse interface dispatch on unbound type arguments (TASK-3388)
Codex round 3: a generic helper run[T](m Mutator[T]) is walked once, with T
unbound, so Mutator[T] never matched *store.Store and the call went
unflagged. An interface call whose receiver is, or is instantiated with, a
type parameter is now refused, rather than propagating type arguments
through the walk. Fixture genericifacebody passes the round-2 checker and is
caught now.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* fix(appstore): no interface dispatch inside a generic body (TASK-3388)
Codex round 4 found two more ways a type parameter hides from the
round-3 check:
- an anonymous interface constraint (or a generic alias)
- a type parameter nested inside a func, slice, map or alias type argument
Chasing type parameters through every type form is open-ended, so the class
is closed outright: a generic function body, walked once with its parameters
unbound, may make no interface method call except on error. A non-generic
function cannot hold an unbound type parameter, so the ordinary
implemented-by-store check stays exact there. Fixtures genericconstraint and
genericnested both pass the round-3 checker and are caught now.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
|
||
|
xarmian
|
4e3988300f |
fix(attachments): a thumbnail takes its original's binding at insert time (BUG-3385) (#1762)
A thumbnail job reads its original once, then decodes and resizes it, and
builds the variant rows from that snapshot. If the original was attached to
an item in the meantime (POST /attachments/{id}/attach), the snapshot's
item_id was still NULL. The attach updates only the variants that already
exist, so the thumbnails were inserted unbound under a bound original, and
orphan GC reclaimed them after the grace period.
CreateAttachmentVariantIfParentLive already locks the parent row before it
inserts. It now reads the parent's item_id under that lock and inserts the
variant with it, so no bind can land between the read and the insert.
|
||
|
xarmian
|
745f576ebc |
ci: run the native PostgreSQL backup/restore round trip (TASK-2962) (#1761)
* ci: run the native PostgreSQL backup/restore round trip (TASK-2962) TestPostgresBackupRestoreConnection had never run in CI. It skips unless PAD_TEST_POSTGRES_TOOLS_URL is set, and CI could not have set it: the Go (PostgreSQL) job's server is postgres:17, ubuntu-latest ships the 16 clients, and pg_dump refuses to dump a newer server. The restore rewrite (#1703) shipped in rc.3 with its round trip unexercised. The job now: - installs postgresql-client-17 from PGDG, after checking the signing key against its published fingerprint, and puts its bin first on PATH - checks that pg_dump and psql are both version 17 - runs the round trip alone and verbosely with PAD_TEST_POSTGRES_TOOLS_URL set, and fails unless the output shows it PASSED. In the quiet suite a skip and a pass look the same, which is how it went unrun. Measured locally against a postgres:17 server, with 17 clients (via docker wrappers): the test passes, and it fails for each of four restore and backup defects: - no --single-transaction: a failed restore left its change committed - psqlrc honoured: a restore reported success after a SQL error - no ON_ERROR_STOP: the same - BUG-2952's PGDATABASE=<uri>: pg_dump failed Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * ci: the PGDG key file must hold exactly one primary key, PGDG's (TASK-2962) Codex round 1: signed-by trusts every key in the file, and the check read only the first fingerprint, so a response carrying PGDG's key followed by another would have passed. The file now has to contain exactly one primary key, and its fingerprint must be PGDG's. Checked locally: the real key is accepted, and the real key with a second key appended is refused. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
cc85c2f9da |
feat(auth): social claim of a never-verified account, provider subjects, cloud-only sidecar routes (TASK-3351) (#1760)
* feat(auth): sidecar OAuth routes are cloud-only; provider subjects bind on next sign-in (TASK-3351) U1: oauth-login, oauth-link and oauth-unlink answer only in cloud mode. A self-hosted install carrying a cloud secret created accounts and minted sessions on a caller's word about an email. U2: user_oauth_identities (SQLite 111 / PG 085) binds a provider's stable account id to one pad account. oauth-login and oauth-link take an optional subject. The first sign-in carrying one binds it, and a different one is refused 403 oauth_subject_mismatch, naming the recovery route (password reset, then relink). A subject bound to another account creates nothing. Unlink forgets the binding. The subject is checked at the door (no NUL trigger), and the four columns are recorded in the NUL baseline. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * feat(auth): a provider that vouches for an address claims the never-verified account (TASK-3351) U3: oauth-login for an existing account whose provider is not linked used to refuse. Now, if the account has never been verified and the provider vouches for the address, it is claimed: - Google and Apple: email_verified is enough. - GitHub: the sidecar must also send email_primary=true (ruling 2), and sidecars today don't, so GitHub claims stay refused until one does. The claim runs the BUG-3382 core, now shared as claimAccountTx: - credentials reset and the epoch bumped - access stripped in workspaces the account doesn't own, and the ones it owns soft-deleted (ruling 3) - an account with billing refused with account_claim_needs_support (ruling 4) The core now also unlinks every sign-in provider and forgets their bound subjects; those were linked by whoever held the account. The claimant's provider is then linked and bound, the display name replaced with the provider's, and a session minted on the claim's epoch. The response carries claimed, stripped_workspaces and deleted_workspaces. The mailbox gets a notice listing them (lead's ruling: the redirect may never render the response); without email it is logged. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(auth): claim, link and bind commit together; serialize provider writes; claim lock order (TASK-3351) Codex round 1: - P1: a social claim committed before its provider subject was checked, so a sign-in refused on the subject left the account claimed. ClaimAccountByProvider now takes the provider and subject and links and binds in the claim's own transaction; a mismatch rolls the whole claim back. - P2: a sign-in that read the link before an unlink could re-bind its subject afterwards. Every provider write (link, bind, unlink, claim) now locks the account row, and a bind requires the provider to still be linked (ErrOAuthProviderNotLinked, answered as oauth_provider_not_linked). - P2: the social claim locked the account before its verification tokens, the reverse of verification, and the two deadlocked on Postgres. A test hook holds the claim at its account lock while a consume starts; without the fix it fails with 40P01 under test-pg, and with it it passes. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(auth): one lock order, account before tokens; a failed provider link stops the sign-in (TASK-3351) Codex round 2: - P2: the token-first order adopted in round 1 deadlocked against account deletion, which locks the account first. Every path now locks the account before its tokens: - deletion (unchanged) - the social claim (back to account-first) - ConsumeEmailVerification and the link claim: each finds the token's account without a lock, locks the account, then spends the token conditionally This also removes an older consume-vs-deletion deadlock. Two tests hold the claim, or deletion, at the account lock while the others start. Under test-pg, the consume-first order fails both with 40P01 and the claim-first order fails the deletion test; the fixed order passes both. - P2: on a new account, a failed provider link was only logged and the sign-in continued, leaving an account that could not sign in with that provider again. Any link failure now stops the sign-in. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(auth): a link claim checks its token's expiry after waiting for the account (TASK-3351) Codex round 3: the spend reused a timestamp taken before the account lock, so a token that expired while the claim waited still claimed the account. The spend now reads the clock itself. Test: a token expiring in 1s, with the claim held at the account lock for 2.1s, claims nothing. It fails with the stale timestamp. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(auth): a claim resets the registrant's username and display name (TASK-3351) Lead's ruling on #1760: the registrant chose the username and display name, and either can impersonate ("support", a staff name). Nothing live depends on the username once the claim has deleted the account's own workspaces. The shared claim core (both the link claim and the provider claim) now resets both, in the claim's transaction under the account lock: - the name: the provider's name, or the address's local part - the username: generated from that name with GenerateUsername, and unique among other accounts The handler's post-claim rename moves into the core. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(auth): a claimed account's username follows the username rules and survives a lost race (TASK-3351) Codex round 5: - P2: the regenerated username could be reserved (support@ gave "support", a provider name "Console" gave "console"). The store cannot import the server's ValidateUsername, so the server installs it on the store (SetUsernameValidator), and candidates that fail it are skipped. - P2: a suffix could push a 39-character base past the limit. Each candidate now cuts the base to fit its suffix. - P2: another account taking the chosen username before the claim wrote it failed the claim with a unique-index violation. The claim now retries, up to three times. Its transaction rolled back whole, the link claim's token included. Tests: reserved and length (store, with a stub validator); the real validator through the social claim (server); the lost race, Postgres only, through a hook that takes the username before the write. Each test fails without its fix. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(auth): a claim falls back to a random username when no candidate from the name will do (TASK-3351) Codex round 6: an email fallback the generator leaves invalid (foo--bar), or a name whose first 1,000 candidates are all held, failed the claim. After the derived candidates, the claim now tries random user-xxxxxxxx names under the same rules and uniqueness check. The test fails without the fallback. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
cd6dfdb318 |
fix(settings): Conventions and Playbooks start unchecked when a member is restricted (TASK-3384) (#1759)
Dave's ruling on TASK-3376's picker default: switching a member from All to Specific collections no longer pre-checks the system collections. They start unchecked, like every other collection. The hint under the list now names the consequence: unchecked, they stay hidden from that member, and their agents won't load the workspace's conventions or playbooks. Migration 108's backfill is unchanged: members restricted before TASK-3376 keep exactly the access they had. A member already restricted keeps their saved list.v0.18.0-rc.4 |
||
|
xarmian
|
98871c7e01 |
fix(auth): a verification link verifies only for its account's session; the mailbox owner can claim (BUG-3382) (#1758)
* fix(auth): a verification link verifies only for its account's session; the mailbox owner can claim (BUG-3382)
Registration mails the verification link to the address, and the address
may belong to someone other than whoever registered it. Clicking it used to
verify the squatter's account in place, keeping the squatter's password and
sessions.
- POST /auth/verify-email now verifies only when the request is signed in as
the account the token names. Otherwise it answers 409 verify_needs_session
with {email, can_claim} and leaves the token unspent.
- New POST /auth/verify-email/claim {token}: the mailbox owner claims a
never-verified account. One transaction spends the token, resets every
credential (an unusable password, 2FA, sessions, PATs, OAuth, CLI
handoffs, reset and verification tokens), marks the address verified,
strips memberships, grants, watches and tabs in workspaces the account
does not own, and soft-deletes the ones it owns (TASK-3351 ruling 3). An
account with a billing customer is refused with
account_claim_needs_support (ruling 4). The answer carries a
set-password path and the affected workspace names.
- users.credential_epoch (SQLite 110 / PG 084): bumped by a password change,
a disable and a claim. Every session mint is fenced on the epoch read
when the credential was checked (login, 2FA via the challenge token, CLI
approval, reset completion, rotation, OAuth login), so a sign-in checked
before a claim mints nothing after it (401 credentials_changed).
- The web verify page offers "Sign in to verify" (back to the link), and,
for an unverified address, a confirmed claim listing what was removed.
- A regression test shows that an unverified account's cookie cannot use
the cookie-fallback routes.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* fix(auth): fence password writes on the checked epoch; own verify bucket; switch account (BUG-3382)
Codex round 1:
- A password change (PATCH /me) or a reset completion that started before
a claim could still overwrite the claim's reset. UserUpdate.ExpectedEpoch
makes the write conditional on the epoch its authority was checked under:
the current password's read, or the reset token's spend. The spend now
reads the epoch in the same statement, and a test hook puts a claim
between the spend and the read-back. A mismatch writes nothing
(credentials_changed).
- verify-email and its claim move to their own EmailVerify bucket (10/hour),
so opening the link, refreshing and claiming no longer spends the
PasswordReset budget the claimant needs to set a password.
- The verify page, signed in as a different account, signs out before
sending the user to sign in. A plain /login link bounced straight back.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
|
||
|
xarmian
|
3bcef0e04d |
fix(access): system collections are ordinary collections for restricted members (TASK-3376) (#1756)
A member restricted to specific collections saw every system collection (Conventions, Playbooks) whether it was listed for them or not, and an editor among them could write it, because requireEditPermission passes any editor before it looks at collections. Eight sites each unioned the system collections in. They now follow member_collection_access and grants like any other collection, for reads and writes. - Store: VisibleCollectionIDsQ, GetUserWorkspaces' freshness MAX and ResolveBacklinksVisibility drop the union; ListSystemCollectionIDs(Q) had no caller left and is removed. - Server: the item-visibility rule, /me's full-access set, watch access, search and the SSE full set drop it. - Migrations 108 / 082 list every live system collection for every member already restricted, so the upgrade changes nobody's access. A system collection created later is not added for them. - Settings: Conventions and Playbooks are real checkboxes in the member access picker, checked by default when an owner moves a member from All to Specific, with a line saying what unchecking them does. Also (PR #1756 codex round 2 P1): an editor or owner ROLE applies only inside the collections a member's access reaches ('all', or listed in member_collection_access). Before, requireEditPermission let any editor through before it looked at the collection, so a restricted editor who could see an item only through a VIEW grant could edit it, comment on it, create beside it and bulk-mutate it. That was true on main for ordinary collections too. Outside the member's reach the grant decides, and only 'edit' writes. The same rule now governs the collab socket's write decision, the bulk per-item and move-target gates, and the cross-workspace edit authorizer, and ResolveUserPermission's membership step checks the member's listed collections instead of the navigation set (which lists a collection because one item in it is granted). The collection check binds the EDITOR role only (codex round 3, lead ruling): an owner can set any member's collection access, their own included, so the owner role keeps its full edit bypass, and the server agrees with the web client's canEditItem / canEditCollection. |
||
|
xarmian
|
0929f65705 |
feat(invitations): a mailbox-only proof on invitation emails verifies the invitee (TASK-3352) (#1757)
* feat(invitations): a mailbox-only proof on invitation emails verifies the invitee (TASK-3352) BUG-3348 stopped an invitation code from verifying the invitee's address, because the inviter sees the code; a cloud invitee then had to click a separate verification email before writing. The invitation email now carries a second secret that only the mailbox receives. - CreateInvitation mints a 128-bit proof next to the code and stores only its SHA-256 (workspace_invitations.proof_hash, migrations 109 / 083). The plaintext is on the returned struct for the email sender only and never serialises. - Both senders (member invite, admin resend, which re-creates the invitation and so rotates the proof) email /join/<code>#proof=<proof>: in the fragment, so it never reaches request logs or a Referer. The invite response's join_url, the members and admin invitation lists and copy-link carry the code alone. - Store.ConsumeInvitationProof spends it in one transaction (user row locked first, live accounts only): clear the hash, mark the address verified. Single-use and bound to its invitation. - Accept (`proof` in the body, after a successful accept, unverified accounts only; the email match already ties the account to the address) and register with an invitation (`invitation_proof`, which then skips the separate verification email) spend it. A wrong or spent proof is ignored, not refused. The code alone never verifies. - The join page reads #proof=, keeps it in sessionStorage for the code across a sign-in round trip, strips it from the address bar and sends it with the accept or signup. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(invitations): an expired invitation's proof verifies nothing (TASK-3352) Codex review round 1: the consume checked only the invitation id and the hash, so a request that passed the handler's expiry check and crossed the expiry before consuming still verified the account (reproduced at 537 ms past expiry). The expiry is now decided inside the consume's own statement. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(invitations): an empty expiry verifies nothing, and a malformed proof fragment is ignored (TASK-3352) Codex review round 2: - expires_at = '' was let through by the consume, though IsExpired reads it as expired; it now verifies nothing, like any expired invitation. - #proof=% made decodeURIComponent throw before the join page's error handling, leaving it loading forever; a malformed escape is now no proof. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * docs(invitations): name the email-match duty of ConsumeInvitationProof's callers (TASK-3352) Lead's review nit. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
82395bde1c |
feat(import): mark rows a workspace import wrote, author not verified (BUG-3379) (#1755)
* feat(import): mark rows a workspace import wrote, author not verified (BUG-3379) Workspace import keeps the export's comment authors, version kinds and attachment uploader ids verbatim (BUG-3372 ruling (1)), and nothing distinguished those rows from authenticated authorship, so an importer could present a fabricated trail. Nothing stored could tell them apart either: user_id is NULL after an account deletion too, the export supplies the timestamps, and import writes no activity. - Migrations 107 / 081 add imported (INTEGER 0/1) to comments, item_versions and attachments. Only the import doors set it (JSON and bundle; a derived thumbnail or transform inherits its original's). - Comments, versions and attachment rows serialise an omitempty imported: true. - An imported attachment's uploaded_by is never resolved to a local name: the member route sends X-Pad-Attachment-Imported: 1 instead. That also closes an importer naming a real member as uploader by putting their id in the manifest. - The web marks them: "· imported" beside a comment or reply author, an Imported chip on a history row (and an imported row never merges with a native one), and "imported" in place of the uploader in the viewer. - Rows imported before this are not marked: there is nothing to backfill from. - MCP tool surface 0.64 (additive): the flag reaches comment and attachment responses on both transports. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(import): keep the imported mark through recovery, autosave runs and copies (BUG-3379) Codex review round 1: - An imported version claiming recovery or system lost its marker in the history; it is kept. - The History tab's autosave row had no marker; it shows the chip. - The server's autosave burst grouping could fold an imported snapshot into a native one (both often have no user id); it no longer does. - A cross-workspace copy carried imported=1 onto a clone the copier uploaded, hiding its real local uploader; the clone is not imported. - A store test inserting attachments by column list is updated for the new column. - The 0.64 changelog names the MCP responses that actually carry the flag: comments, attachment list and full history (not history summaries, attachment show or the attachment resource). Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(import): a local edit never lands inside an imported version (BUG-3379) Codex review round 2: the version throttle ignored the flag, so a local edit within the hour, by the same kind and source as the newest imported version, wrote no version and read as part of the imported change. An imported predecessor now always starts a new version. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
4d6ebd426c |
fix(server): attribution comes from the request, never the body (BUG-3372) (#1754)
* fix(server): attribution comes from the request, never the body (BUG-3372) A body could set who wrote a comment, an item, a link or a version, and from which client: author (free text, rendered as the comment's author), created_by, source, last_modified_by and version_source all yielded to a body value, so any editor could post as another member, or file a write as an agent's or a human's at will. - Comments and replies: author is the signed-in account's name (empty, labelled by kind, with no account); created_by and source are the request's. - Item create: created_by and source are the request's. Item update: last_modified_by and the version row's source are the request's, and a body source no longer moves items.source (an update never should). - Links (blocks / parent / ...): created_by is the request's. - The canonical account columns are filled on every request write path: items.created_by_user_id and last_modified_by_user_id (create, update, artifact import, cross-workspace copy), item_links.user_id (link create, parent set on create and update). Account deletion already nulls all three. - X-Pad-Agent stays the agent signal (self-declared; it only labels the caller's own writes) but is trimmed, stripped of control characters and capped at 64 runes; a header that cleans to nothing is not an agent. No client sends any of these fields except the web's comment create and the MCP comment door, which send the values the server derives anyway. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(server): legacy document doors and bulk edits stamp attribution from the request (BUG-3372) Codex review round 1: - POST/PATCH /documents still took created_by, source and last_modified_by from the body; they are now the request's. - Bulk field and tag edits stamped the writer's kind but not the account; they now fill last_modified_by_user_id like a single update. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(server): the document autosave hint reads the body's source before attribution is stamped (BUG-3372) The legacy document PATCH skips its activity row for a content-only web autosave, keyed on the body's source. Stamping source from the request made every cookie request 'web', so every content-only update lost its activity row (TestActivityEndpoints). The body's value is kept as that hint only: it decides whether an activity row is written, never what any row says. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(server): the document autosave skip also requires a web request (BUG-3372) Lead's review: the autosave hint is the body's, so any caller could skip the activity row; a PAT or agent sending "source":"web" on a content-only PATCH wrote no audit record. The skip now also requires the request's own source to be web. Red-first test with the cookie control. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
db62001ad4 |
fix(billing): a plan past its expiry entitles nothing, and Stripe plan writes are ordered (BUG-3356) (#1753)
* fix(billing): a plan past its expiry entitles nothing, and Stripe plan writes are ordered (BUG-3356)
/admin/plan was last-write-wins under the plan-source rule, so two
Stripe-derived writes arriving out of order could leave the older one
standing and restore an entitlement a newer one removed; and
plan_expires_at was stored but never read.
- models.User.EffectivePlan / PlanExpired: an expiry at or before now
(or one that does not parse) makes the plan free. Every entitlement
reader uses it: the workspace and user limit checks and workspace
storage info; the session and account responses report it. Admin
responses keep the stored plan and add plan_expired.
- Migrations 106 / 080 add users.plan_revision and
plan_subscription_id. /admin/plan takes optional revision (the
sidecar's fetch time, unix ms) and subscription_id; a write with a
revision applies only if it is newer than the stored one, decided in
the same UPDATE as the source rule. A refusal answers 200
applied:false with reason "source" or "stale_revision".
- A stripe write with no revision (a sidecar that predates it) still
applies, unordered, and is logged and counted in
pad_plan_writes_total{outcome="no_revision"}; a later change refuses
it once the sidecar sends one.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* fix(billing): bigint revisions, manual writes fence in-flight Stripe writes, expiry-aware billing counts (BUG-3356)
Codex review round 1:
- Revisions are unix ms, beyond int4; Postgres inferred int4 for the
bare placeholders compared with 0 and refused real values (500).
Every revision placeholder is now CAST AS BIGINT; a test uses real
millisecond values and fails on Postgres without the casts.
- An admin downgrade left the stored revision alone, so a Stripe fetch
taken before it and landing after it restored pro. A manual write now
raises plan_revision to now (unix ms): a fetch from before the
operator's decision is refused as stale, one from after still applies.
Pad and the sidecar share a host, so the two clocks are close; skew
moves the fence by that much.
- The admin billing counts (customers by plan, new pro signups, the
admin user list's plan counts) ignored expiry; they count the
effective plan, decided in Go because a stored RFC3339 value may carry
an offset and does not sort against now.
- A refusal's reason was read after the UPDATE in a fresh snapshot; the
row is now read under its lock first and both decisions use it.
- subscription_id is validated at the door (Stripe-shaped, at most 255
bytes), since the column takes the NUL baseline's exemption.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* fix(billing): a manual write cannot carry a revision, and the admin update validates expiry (BUG-3356)
Codex review round 2:
- /admin/plan accepted a revision on a manual write, which replaced the
current-time fence with a value the caller chose; a revision is now
refused (400, and by the store) unless the source is stripe.
- The admin user update stored an expiry without parsing it; under
expiry enforcement an unparsable one stores pro and grants nothing.
It is now refused as /admin/plan refuses it.
- The over-length subscription id case used NUL bytes, so JSON
rejection masked the length check; it is now a valid-shaped id.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
v0.18.0-rc.3
|
||
|
xarmian
|
a2e1c5d9ca |
fix(oauth): narrow current-only wildcard connections to their current workspaces (BUG-3338) (#1752)
Connected Apps let a user turn include_future_workspaces off under the wildcard, but the wildcard is live and the flag was never read, so those connections kept reaching workspaces joined later. A startup step, next to BackfillOAuthConnections, turns each such connection into a specific list of its user's live memberships and clears the wildcard: the same result as "Limit to my current workspaces". Narrowing-only (it writes rows only for workspaces the wildcard already reached and only ever clears the wildcard), idempotent (a narrowed row no longer matches, and no door stores the split combination since BUG-3338), one transaction, and it logs how many connections it narrowed, how many rows it wrote, and how many were left empty. A Go step rather than a numbered migration because the migration runner has no per-migration hook to log a count. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
Matt Faltyn
|
5614980edc |
Make database restore preserve live data on failure (#1703)
Stage and normalize SQLite backups before atomic publication, checkpoint committed live journals safely, and stop PostgreSQL restores on SQL errors. Cover recovery failures and compatibility with real-engine regressions. |
||
|
xarmian
|
5749671dd9 |
fix(oauth): include_future_workspaces is no longer a control; limit to current is an action (BUG-3338) (#1751)
* fix(oauth): include_future_workspaces is no longer a control; limit to current is an action (BUG-3338)
The wildcard (all_current_workspaces) is live and covers workspaces
joined later, and nothing ever auto-added on include_future_workspaces,
so the Connected Apps checkbox promised something false either way:
unticked under the wildcard it still granted workspaces joined later,
and ticked on a specific list it added nothing.
- Connected Apps drops the checkbox and labels the wildcard "All my
workspaces, including ones I join later".
- PATCH /connected-apps/{id}/flags ignores include_future_workspaces
(an older page may still send it); the stored value follows
all_current_workspaces, as the consent screen always wrote it.
- POST /connected-apps/{id}/limit-to-current copies the user's live
memberships into the list and turns the wildcard off in one
transaction: the explicit form of "only my current workspaces".
- The consent screen is unchanged.
Rows already stored with the wildcard on and include_future off are
narrowed by a separate migration, held for a production count.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* fix(oauth): limit acts only on a wildcard, keeps guest workspaces, and the empty-list rule holds under the connection lock (BUG-3338)
Codex review round 1:
- A repeated limit (a stale tab) on an already specific list added the
workspaces joined since; it now changes nothing unless the wildcard
is on.
- The snapshot left out workspaces the user reaches only as a guest
through a collection or item grant, which the wildcard reaches; it
now includes them.
- Removing a workspace and turning the wildcard off checked the
empty-list rule outside any lock, so a concurrent limit or flags
change could leave a specific list empty. Both now check and write
in one transaction holding the connection row's lock.
- Turning the wildcard off and limiting now re-check live connections,
as a removal does (TASK-3365).
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* fix(oauth): the limit snapshot counts only live guest grants (BUG-3338)
Codex review round 2: a grant on a soft-deleted item or collection
confers no access, but put its workspace in the snapshot, where it
counted toward the not-empty rule and would come back to life if the
content were restored. The grant subqueries now use the live-grant
filter UserHasGrantsInWorkspace uses.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
|
||
|
xarmian
|
119823d62d |
fix(server): role-board reorder refuses another workspace's item and counts its rows (BUG-3342) (#1750)
The reorder loaded each item with the global GetItem and never compared its workspace with the route's, so an item from another workspace the caller could edit passed every check. The workspace-scoped UPDATE then matched nothing and the answer was 200, which also told the caller the id existed somewhere. - The handler answers a foreign item exactly as a missing one. - UpdateRoleSortOrder requires each entry to update exactly one live item in the workspace, or rolls the whole batch back with ErrRoleSortItemNotFound, which the handler answers the same way. - checkItemVisibleQ refuses an item whose workspace is not the one asked about, as defence in depth. All 61 existing callers already pair the two. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
23a0360991 |
fix(mcp): a connection row decides workspace access, not the legacy token list (BUG-3337) (#1749)
* fix(mcp): a connection row decides workspace access, not the legacy token list (BUG-3337) The MCP OAuth gate unioned a grant chain's legacy session.Extra allow-list with its oauth_connections rows. Startup backfill seeds a row from Extra for every pre-PLAN-1519 chain, and Connected Apps edits only the row, so narrowing such a connection (wildcard to specific, or removing a workspace) never took effect, and refresh carried Extra forward so it never would. With a row present the row is authoritative and Extra is ignored. Extra still decides for a chain with no row (a backfill that failed for it), as before. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * docs(mcp): name the restored-workspace exception the connection gate leaves (BUG-3337) Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
681780486f |
feat(oauth): count and log requests that omit the RFC 8707 resource (TASK-3363 phase 1) (#1748)
Dave's ruling on TASK-3345 #1: a missing `resource` parameter is defaulted to /mcp's canonical audience, which lets a client registered through DCR replay a token cross-server. Phase 2, a release later, refuses such requests with invalid_target. Phase 1 measures who that would break, and changes no behaviour. - translateResourceToAudience now reports whether it DEFAULTED a missing resource. The authorize, decide and token handlers then call noteResourceMissing. - noteResourceMissing logs one WARN line naming the endpoint, client_id and the client's registered name, and increments the new pad_oauth_resource_missing_total{endpoint}. The label is the endpoint only, so cardinality stays bounded; the client is in the log. - Legs: with resource, nothing is counted; without, the request is still served and counted at authorize, decide and token. Mutants L1-L5 killed. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVCv0.18.0-rc.2 |
||
|
xarmian
|
67d06fdf81 |
fix(auth): a CLI session is never a browser cookie, and sign-in refuses a cross-site form (BUG-3350) (#1747)
(a) Sessions did not record how they were issued, and SessionAuth accepted
any padsess_ token from a cookie as a cookie session. isBearerAuth is
decided by transport, so an admin's CLI session sent as a Cookie picked up
the cookie-only platform-admin bypass that bearer use is denied (BUG-1616).
- Migration 105/079 adds sessions.kind ('web' | 'cli'). It backfills 'cli'
from device_info 'cli-browser-auth' (lead's ruling: no forced re-login;
older CLI password logins stay 'web' until they expire).
- The kind comes from the device label at creation.
- The CLI sends X-Pad-Client: cli, so its password and 2FA sign-ins mint a
'cli' session and set no cookie. Rotation keeps the replaced session's
kind.
- SessionAuth, validateSessionCookie and requestSessionInfo's cookie path
accept only a 'web' session as a cookie. A CLI session still works as
the bearer it is.
(b) /auth/login (and 2FA login-verify) were CSRF-exempt and decoded JSON
whatever the Content-Type, so a cross-site text/plain form could sign the
victim into the attacker's account. A sign-in must now be
application/json (415 otherwise), and an Origin the browser names must be
this server (its Host or base URL) or an allowed CORS origin, including the
localhost defaults the dev server uses (403 otherwise). Headless clients
send no Origin. An E2E leg pins the web app's own login form sending JSON.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
|
||
|
xarmian
|
56922e790a |
feat(server): every access and credential change re-checks live connections (#1746)
* feat(server): every access and credential door kicks live connections (TASK-3365 PR2) PR1 (#1744) kicked through the doors that already publish workspace_access_changed. This wires the rest: member role change and collection access; grant creation (which can narrow access); collection delete and item archive/move/restore, single and bulk; logout (each deleted session's owner), the session a sign-in replaces, strict-mode IP/UA kills; password change/reset/local reset and TOTP disable through rotateSessionsAfterCredentialChange; admin role change, temp password and disable; account delete, including the workspaces of grants the user issued, reported from DELETE ... RETURNING inside the deletion transaction; PAT delete/rotate, workspace-token delete; connected-app revoke and workspace removal; unusable-workspace cleanup. A source census fails any function that calls an access- or credential-changing store method without kicking. * test(server): account deletion kicks the workspaces of grants it issued (TASK-3365 PR2) The lead asked for the door to be proven rather than left to review: a freshly signed-in interactive session deletes an account that issued a grant in someone else's workspace, and that workspace's connections are kicked. Red with the kick loop removed. |
||
|
xarmian
|
2bd0862af2 |
fix(auth): platform administration refuses an API token (BUG-3361) (#1742)
* fix(auth): platform administration refuses an API token (BUG-3361)
BREAKING for anyone driving /api/v1/admin/* or admin account creation with
a personal access token: those now answer 403 session_required. Use a
`pad auth login` CLI session, or on self-host set PAD_ADMIN_API_TOKENS=allow.
An admin's PAT carried full platform authority. codex reproduced the chain
during BUG-3336's review: register an account with a chosen password,
promote it to admin, sign in as it, and reset any account's password. A
leaked admin token therefore escalated to interactive sessions for any
account.
- refuseAPITokenForAdmin guards the whole /api/v1/admin route group, so an
admin route added later is refused by construction.
TestBUG3361_EveryAdminRouteRefusesAnAdminPAT walks the router to hold it.
- handleRegister refuses admin-create signup with an API token.
- PAD_ADMIN_API_TOKENS=allow is the self-host escape hatch. It is read at
startup, logs a WARN, and is never honoured in cloud mode. Documented in
docs/deployment.md, Security.
- A browser session and a CLI session (padsess_) are unaffected. So is the
Pad Cloud sidecar, which authenticates with the cloud secret, never a
PAT. /audit-log already refused a PAT (BUG-3334).
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* test: drive platform administration with the admin's session, not its API token (BUG-3361)
CI on the first push: 70 E2E specs and TestMCPCapability_UpgradeChangesNothingWhileOff
called /api/v1/admin/* or admin account creation with an admin PAT, exactly
the use this change refuses. Our own harness was the first operator script
the breaking change broke.
- E2E: global setup keeps the session token login already returns as
fixture.adminSessionToken (padsess_ is not User-Agent bound when sent as
a Bearer). The seven admin call sites use it: account creation in
permission-walk, workspace-tabs, workspace-access-live, no-commerce and
account-delete; the admin user search; and PUT /admin/mcp. Every other
call keeps the PAT, including the MCP client config in plan-2310.
- mcp_capability_test: the admin drives /admin with a CLI session and keeps
its PAT for /mcp, which takes one.
Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC
* test(e2e): the admin console specs use the admin's session (BUG-3361)
CI on
|
||
|
xarmian
|
e515da41ee |
docs(chatgpt): state the personal-data policy as 'Pad adds no personal data' (TASK-3364) (#1745)
Dave's ruling on TASK-3345 #2. What Pad attaches to a ChatGPT response (internal ids, account emails, an email-shaped display name) is stripped; what users wrote (titles, bodies, comments, field values) passes through unchanged. The stated invariant in chatgpt_project.go, the catalog doc comment and the plugin's longDescription now say exactly that. No behaviour change. |
||
|
xarmian
|
484ca668a3 |
feat(server): revoking access re-checks live connections immediately (#1744)
* feat(server): access kicks re-check live connections now, not on the next tick (TASK-3365 PR1) Dave's ruling on TASK-3345 #3. A per-instance registry indexes live collab, /events and /events/stream connections by user and workspace; a kick runs the revalidation each already performs on its 60s tick immediately (the tick stays as the backstop). Kicks travel the watch bus as the server-internal KindAccessInvalidated, which watchStreamDelivers drops, and the existing workspace_access_changed publication kicks too, so the doors that already publish it are covered. Kicks are non-blocking sends that coalesce per connection. PR2 wires the remaining access and credential doors. * fix(server): access kicks ride their own transport, reach watch streams, install idempotently (TASK-3365 PR1) Codex r1: kicks on the client watch bus took sequence ids, replay slots and client queue space, so they could evict real notifications and open gaps. They now ride internal/accesskick (Redis pub/sub on its own channel, or local with one instance), and the access-changed helpers kick directly, so nothing internal touches the client bus and the internal kind is gone. Watch streams register under a wildcard workspace so workspace kicks reach them. SetAccessKickTransport is idempotent and replacement stops the old subscription. A partial revoke kicks too. Tests make the first tick deterministic. * fix(server): access kicks never wait on Redis on a request path (TASK-3365 PR1) Codex r2: the shared Redis client ignores a context deadline on command I/O, so a stalled Redis held each publish ~5s, serially per recipient, on the request that revoked access. Local connections are now kicked at once and remote publication goes through one bounded background worker (dropped when full; other instances fall back to the tick); the transport uses its own client clone with context timeouts enabled; lifecycle changes send one workspace kick, and a complete revoke no longer kicks twice. * fix(server): access-kick worker cancels on removal; transport owns and closes its client (TASK-3365 PR1) Codex r3: the worker published with context.Background, so removing a stalled transport left the worker and queue alive; it now has its own cancellable context. The transport's cloned client is closed when the server replaces or clears it, and the clone gets its own push-notification processor instead of registering on the source client's. |
||
|
xarmian
|
0f5b567fe7 |
fix(server): lineage and closure name only items the caller may see (#1743)
* fix(server): lineage and closure name only items the caller may see (BUG-3366) Dave's ruling on TASK-3345 #4: HIDE. Parent title/ref and derived-closure related items authorized the other item by its COLLECTION, which is navigation-lenient for item grants, so an item-grant guest was shown ungranted parents and superseders in a granted item's collection. They now use the per-item policy relation_targets uses (itemVisibleFor, shared and fail-closed), for restricted callers in the list, single-item and closure paths. TestProjectChangelogEndpoint_GuestParentFilter_ItemGrantOnlyCollection is flipped: an ungranted parent is not matchable, a granted one is. * fix(server): redact an ungranted legacy relation target by title too (BUG-3366) Codex r1: a target with item_number NULL hydrates with a title and no ref, and redaction keyed on the ref alone, so an item-grant guest saw an ungranted legacy target's title through a granted item's relation field. |
||
|
xarmian
|
66b4a9d477 |
fix(auth): disabling an account ends every credential, and a disabled account gets no new one (BUG-3349) (#1741)
(b) Admin disable deleted sessions only, so the account's PATs and MCP grants kept working. DisableUserAndRevokeAccess does all of it in one transaction: - disables the account; - deletes its sessions and API tokens; - deactivates every OAuth access and refresh token (by subject), and every unexchanged authorization code (through its connection's request id); - deletes its PKCE and OAuth connection rows. EnableUser restores none of them. (c) A disabled account acts nowhere and gets no credential: - requireActiveUserTx is the one gate every credential insert passes. It reads the user row in the insert's own transaction, FOR SHARE on Postgres, and refuses a disabled or missing account with ErrUserDisabled. It covers sessions, API tokens, OAuth codes, access and refresh tokens, and OAuth connections. FOR SHARE conflicts with the disable's row lock, so a mint admitted before a disable cannot land after it. TestBUG3349_MintWaitsForAConcurrentDisable_Postgres proves the wait with two connections. TestEveryCredentialInsertRequiresAnActiveUser fails on any new credential insert that skips the gate. - RequireAuth checks disabled status before the /api/v1/auth/* exemption. Only POST /auth/logout and GET /auth/session stay open, and the session check answers account_disabled. - The MCP door, /oauth/token, /oauth/authorize and /authorize/decide, 2FA login-verify, and email verification (before its token is spent) refuse a disabled account. The login page shows "Your account has been disabled". - Share links the account created are suspended (lead's ruling): byte-identical to an unknown token while it is disabled, serving again once it is re-enabled. Webhooks are unchanged by design; they belong to the workspace. This is documented in docs/deployment.md, "Disabling an account". Part (a) shipped in BUG-3336. codex review: two rounds, eleven findings, all fixed. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
9ef346268c |
fix(import): refuse duplicate source ids and attach by source id (#1740)
* fix(import): refuse duplicate source ids before writing anything (BUG-3357) The import remaps collections and items by source id, and a bundle's attachments by attachment id. Two rows sharing an id collapsed into one mapping, so every reference to it (an item's collection or parent, links, comments, versions, reminders, relation fields, pad-attachment refs) landed silently on whichever row was written last. The store now refuses a duplicate collection or item id with a validation error before its transaction opens (both import doors answer 400), and the bundle door refuses a duplicate attachment id in the manifest, rolling back the workspace pad-export.json minted. * fix(import): attach by source-id map, refuse repeated blobs and colliding empty ids (BUG-3357) Codex r1: the bundle door attached rehydrated attachments through the item SLUG, which the import renames when two items share one, so an attachment landed on the wrong item; it now uses the store's source-id -> new-id map (ImportReport.ItemIDs). A second tar entry at an already-rehydrated blob path is refused. Two rows without an id collide on "" exactly like two with the same id, so the second is refused; a single id-less row stays importable. * fix(import): ItemIDs names inserted items only; check a repeated blob first (BUG-3357) Codex r2: ItemIDs carried items the import skipped (missing collection), whose ids exist nowhere; it now returns the inserted-only map. The repeated blob check ran after the per-blob size check, so an oversized repeat took the keep path; it now runs first. |
||
|
xarmian
|
9e886ddc00 |
fix(web): a link that opens elsewhere never keeps window.opener (BUG-3359) (#1739)
Raw HTML in markdown bypasses the safe link renderer, and both sanitizers
kept the author's target and rel, so <a target="_blank" rel="opener">
reached {@html} with window.opener live in the new tab. An
afterSanitizeAttributes hook in sanitizeMarkdownHtml and sanitizeHtmlBlock
now drops the opener token and forces noopener noreferrer on any element
whose target is not _self, keeping the author's other rel tokens.
|
||
|
xarmian
|
c52f64e263 |
fix(netpolicy): one outbound address policy for URL import and webhooks (#1738)
* fix(urlimport): one address policy for URL import and webhooks (BUG-3358) URL import refused only loopback, RFC 1918, link-local and CGNAT, so it reached 198.18.0.0/15, 0.0.0.0/8, 240.0.0.0/4 and the other reserved ranges webhooks already refused, and the import door returns the fetched body. Both now call internal/netpolicy.Blocked, checked at dial time against the address being connected to, which also adds IPv6 forms that embed IPv4 (NAT64, 6to4, IPv4-compatible) and the remaining special-purpose IPv6 blocks. Regression legs cover a literal address, a DNS answer and a redirect. * fix(netpolicy): registry gaps, public exceptions, NAT64 by embedded address, malformed input (BUG-3358) Codex r1: add 192.88.99.0/24, 100:0:0:1::/64, 3fff::/20 and 5f00::/16; allow the globally reachable exceptions inside 192.0.0.0/24 and 2001::/23; judge the NAT64 well-known prefix by the IPv4 address it embeds instead of refusing the prefix; refuse any slice that is not 4 or 16 bytes. The redirect test now requires the policy refusal, and the dialer's literal screen has its own leg. |
||
|
xarmian
|
d212eb6503 |
fix(members): keep a workspace's owner and at least one owner member (#1737)
* fix(members): a workspace keeps its owner and at least one owner member (BUG-3355) Role changes and member removals checked nothing about ownership: an owner could demote themselves or remove the last owner, leaving zero owners, and demoting the canonical owner split authority from workspaces.owner_id, which sets the URL, restore rights and plan limits. The store now refuses, in the write's own transaction, demoting or removing the canonical owner (409 canonical_owner) and leaving no owner member (409 last_owner), locking the owner membership rows FOR UPDATE on Postgres. An unknown role is a 400 instead of the CHECK constraint's misleading 404. * fix(members): anchor the owner invariant on the canonical owner, without row locks (BUG-3355) Codex r1: locking the other owners' rows FOR UPDATE neither serialized a concurrent promote-then-demote under READ COMMITTED nor kept a lock order account deletion agreed with (40P01). The canonical owner is an owner member in every live workspace and owner_id cannot change, so refusing that one row keeps an owner whatever runs concurrently; the last-owner count stays as a best-effort guard for legacy rows. Store test covers all three doors, including removal without revoking grants. |
||
|
xarmian
|
3fe40aeff7 |
fix(import): bound the work a single bundle import can do (#1735)
* fix(import): bound a bundle's decompressed bytes and entry count (BUG-3354) The 2 GiB cap applied to the compressed body only. The tar stream now counts every decompressed byte against 4x the body cap plus room for both metadata files, refuses an entry declaring more than the budget has left before inflating it, and stops after 100,000 entries. Each answers 413 bundle_too_large through the existing reject path, which rolls back a workspace the bundle had already minted. * fix(import): charge sparse entries logically and count extension header blocks (BUG-3354) Codex r1: a sparse entry's holes are synthesized above the counted stream, and PAX/GNU extension headers are consumed inside Next where the entry count cannot see them. Declared sizes now draw on their own logical budget, the header blocks each Next reads are capped, and skipped non-regular entries are drained by the loop so that count stays headers only. * fix(import): refuse a bundle entry with a negative declared size (BUG-3354) Codex r2: archive/tar accepts a negative base-256 size on some entry types, and charging one raised the logical budget it was charged to. * fix(import): enforce the tar header limit inside each Next call (BUG-3354) Codex r3: tar.Reader.Next walks a whole PAX/GNU extension chain before it returns, so a header-block limit checked afterwards refused the bundle only after all of that work. The counting reader now carries a header window armed around each Next and fails the read that would pass it. * fix(import): the decompressed-size 413 states its limit and the setting that raises it (BUG-3354) |
||
|
xarmian
|
551a338cbd |
fix(auth): a user-less legacy workspace token reaches only its own workspace (BUG-3353) (#1734)
A legacy workspace token (an API token with no user) carries one workspace and nobody's authority, but RequireAuth admitted it on every route. On global routes it acted with no user: - POST /workspaces minted a workspace with any slug, attributed to the body's owner_id, with no plan limits and no member row. That let it plant workspaces on a victim and fill their cloud quota. - POST /workspaces/import minted unowned workspaces without limit. - GET /workspaces fell into the fresh-install branch and listed every tenant on the instance. The fix is deny-by-default. RequireAuth admits such a token only on workspace-scoped routes, where RequireWorkspaceAccess pins it to its own workspace, and on a short allowlist of global routes that are either confined to the token's workspace (GET /workspaces, /search, /events, /collab) or static. Anything else answers 403 legacy_token_not_allowed. TestBUG3353_EveryGlobalRouteRefusesALegacyToken walks the router, so a route added later fails closed by test. - GET /workspaces with such a token lists its own workspace only. The no-user bootstrap branch now needs a user count that reads zero. - Minting: the owner is always the authenticated user and a body owner_id is ignored. beginWorkspaceMint refuses a user-less caller unless the instance has no users and there is no token. That settles the open question BUG-2809's trail left. - A token pointed at a foreign workspace now answers exactly what a missing one does: the workspace 404 on RequireWorkspaceAccess and /events, and the item 404 on collab. The 403 told the token which slugs and items exist. (Real users' BUG-3069 contract is unchanged, per the lead's ruling.) - TokenAuth never downgrades a user-owned token to a user-less one: a user that cannot be read is a 500, one that is gone a 401. Before, the request went on with only the token's workspace, which is a legacy token's reach. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
3d21213b57 |
fix(account): account deletion needs an interactive, recently signed-in session (BUG-3336) (#1733)
* fix(account): an API token cannot delete the account; confirm-only is for passwordless cloud accounts (BUG-3336) POST /auth/delete-account deletes the user and soft-deletes every workspace they own, and two holes let it go through with less proof than the caller should have to show. 1. Confirm-only (no password) was accepted from ANY cloud account. The path exists for OAuth-only accounts, which have no password to give, but it never checked HasPassword, so a cloud session alone could delete an account that has one. It now requires !HasPassword. This matches the web client, which offers confirm-only only when password_set is false. 2. A PAT was accepted like a session, so a leaked write token could delete the account (and, with the password, on self-host too). An API token is now refused with 403 session_required, before the body is read, so it cannot probe the password either. This is the line BUG-2890 drew for token minting. A browser or `pad auth login` CLI session still works. No CLI or MCP command deletes an account. TOTP is unchanged and still runs after either identity check. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(account): confirm-only deletion needs a recent sign-in; rotation keeps the sign-in time (BUG-3336) codex review of the first commit: password_set=false does not prove an account has no password. An account that signed up with a password before migration 043 and later linked OAuth reads false and still holds a real hash, and CreateOAuthUser stores a hash too (a random placeholder), so the data cannot tell the two apart. A session alone could still confirm-delete such an account. Confirm-only now also needs a session signed in within accountDeleteReauthWindow (10 minutes, measured from sessions.created_at). Otherwise it answers 403 reauth_required, "For your security, sign in again to delete your account", and the settings page offers a "Sign in again" button: it signs out and returns to /login?redirect=/console/settings, because /login sends a signed-in visitor straight on. A credential-change rotation (password change, 2FA, OAuth unlink) no longer restarts the clock. CreateSessionIssuedAt gives the new session the replaced one's created_at, which also stops a rotation from extending SessionMaxLifetime (measured from created_at); its expiry is capped there. ValidateSession now returns CreatedAt. TestWorkspaceAccessChanged_AccountDeletion deleted through a PAT, the path the first commit refuses; it now uses an interactive session. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(auth): no session-minting door hands out a fresh sign-in without one (BUG-3336, BUG-3349a) codex review, round 2: the re-auth window could be beaten by minting a session whose created_at reads as new. - CLI approval minted a 30-day session from any authenticated caller. It now refuses an API token (403 session_required), and the CLI session it mints carries the APPROVING session's sign-in time (CreateSessionIssuedAt). An approver whose session cannot be read is refused. - requestSessionInfo read `Bearer padsess_` literally and missed a padded header that TokenAuth accepts after trimming. It now reads the header the way TokenAuth does, and a bearer of any kind is the credential. - Rotation (rotateSessionsAfterCredentialChange) fell back to a fresh created_at when it could not find the request's session. Every caller is a session-only door, so it now revokes every session and asks for a new sign-in instead of minting one. - oauth-unlink and PATCH /auth/me with a password change refuse an API token, so rotation is reachable only from a session (TOTP disable already was). A PAT can still edit the profile. These three gates are BUG-3349's part (a), moved here because the re-auth window depends on them (lead's ruling). Legs: CLI approval from an old session or a PAT, the padded bearer, rotation with no session, and the SessionMaxLifetime cap on rotation. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC * fix(auth): admin password reset refuses an API token; the request's session is the one that authenticated it (BUG-3336) codex review, round 3. - An admin's PAT could reset its own account, get a temp_password back, and sign in as a fresh session. handleAdminResetPassword now refuses an API token (403 session_required). - requestSessionInfo re-read the Authorization header. An invalid bearer on /auth/* falls through to the cookie in TokenAuth, so a valid cookie session beside a stale PAT header read as "no session": CLI approval answered 401, and the rotation doors revoked every session and answered 500. It now follows ctxAuthKind, the credential the middleware actually used (as credentialLiveness does): a session bearer is read from the header, an API token is no session, and anything else from the cookie. - Legs added: - the password-change, oauth-unlink and admin-reset PAT gates, each refusing with nothing changed or revoked behind the refusal; - a stale bearer beside a valid cookie; - a rotation with no session revoking every session. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
4b53142589 |
fix(server): match accounts by email only once the address is verified (#1732)
* fix(server): by-email access doors require a verified account (BUG-3348) * fix(server): an invitation code does not verify an address (BUG-3348) The inviter receives the same code in the invite response, so accepting by code no longer verifies the account, and on cloud with a deliverable verification email an invited signup starts unverified and is sent the link. Self-hosted is unchanged. * fix(server): mint the verification link before an invited signup's membership writes (BUG-3348) Codex r2: after the membership row exists DeleteUser cannot roll the account back, and the invitation path's kept-account return skipped the later send. |
||
|
xarmian
|
68a7e279a7 |
fix(leases): bind item leases to the claiming account (#1731)
* fix(leases): claiming takes edit permission, and a lease is bound to its claimer (BUG-3341) Item claim and release checked only that the caller could SEE the item, so a viewer or a view-only guest could claim an item (blocking others from working it) or release someone's lease. Both now take requireEditPermission, grant-aware like any item edit. The holder is a free-form label (an agent name, "sweep-runner") and it alone authorized refresh and release, so any editor could end or extend another user's lease by naming its label. Migration 104 (SQLite) / 078 (Postgres) adds items.lease_user_id: a claim records the signed-in user, and refresh and release require the same user AND the same label; any other caller gets the existing 409 lease_held. Leases claimed before the migration have no user recorded. They keep refreshing and releasing on their label alone until they expire (at most 24h), so a deploy breaks no live lease; the first refresh by a signed-in caller binds it. Tests: a viewer and a view-only guest are refused, an editor and an edit-grant guest allowed; another editor naming the label can neither release nor refresh; a pre-binding lease works on its label and binds on refresh. Each goes red with its fix removed. Co-Authored-By: Claude <noreply@anthropic.com> * fix(leases): classify items.lease_user_id in the NUL-column census (BUG-3341) Codex (no P0/P1, no lease-authorization defect) P2: the new column joined the schema without a census entry, which TestNULColumnCensus refuses. It holds the authenticated caller's server-generated user id, written only by ClaimItemLease from the session, so it is recorded as an exclusion with that reason. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
xarmian
|
1f391d9591 |
fix(auth): store errors on access decisions fail closed (BUG-3334) (#1730)
A user count that cannot be read is no longer an empty instance. The fresh-install bypass is granted only when the count reads zero WITHOUT an error. Before this, RequireAuth let an anonymous request through, RequireWorkspaceAccess granted owner, CSRFProtect skipped its check, the SSE recheck answered "everyone has access" and collab granted write. On error each now goes on to its normal principal checks, so a signed-in caller keeps working through a count fault and an anonymous one is refused. Six item handlers ignored a visibleCollectionIDs error and built the response with nil, which means unrestricted, so a restricted member was shown a hidden parent's title (get, update, restore) and progress counted hidden children. - get and progress now answer 500. - update and restore resolve visibility BEFORE the write, so an error is a 500 with nothing written, not a committed write reported as failed (create has no idempotency key). - create and move reuse the resolution they already checked before their write. /audit-log refuses a PAT with 403 session_required. The platform-wide log belongs to the admin's interactive session (cookie or CLI session, so `pad workspace audit-log` keeps working), the line BUG-2890 drew for token minting. Legs inject real faults where one can reach the site (a renamed table). Two test-only seams cover the sites an earlier check masks: - visibleCollectionIDsFault: requireItemVisible runs the same query first. - userCountFault: the workspace lookup also reads users. Both are nil in production, which a test asserts. Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC |
||
|
xarmian
|
1f982df402 |
fix(sse): an item event without a collection is not workspace-level (BUG-3343) (#1729)
The comment-update, reply and reaction publishers resolved the item's collection AFTER the write with GetItem, which skips archived items, so an archive racing the write left the event's collection empty. The SSE filter treats an empty collection as a workspace-level event and delivered it to every member, so a collection-restricted member received the hidden item's ID and title. - sseEventVisibleFor: for a restricted subscriber, an item event with no collection is refused (it names one item, and there is no way to confirm it is visible). Workspace-level events without an item are unchanged. - The three post-write lookups use GetItemIncludeDeleted, so the event keeps its collection and still reaches those who may see it. Each test fails with its own fix reverted. Co-authored-by: Claude <noreply@anthropic.com> |
||
|
xarmian
|
30ba2ed304 |
fix(api): body-supplied IDs must name something in the authorized workspace (BUG-3346) (#1728)
Three request-body IDs were never checked against the workspace the route authorized: - A top-level comment's parent_id: the store's parent check was `WHERE id = ?`, so a comment in one workspace could hang off another workspace's comment (an existence oracle on its IDs, a changed delete behaviour for its owner, and a foreign key that can block its purge). The check now requires the parent to be on the same item in the same workspace; anything else is the not-found answer, as the reply route already enforced. - An item's raw top-level parent_id on create and update, written to items.parent_id as given (the store trusts its caller to have checked). It must now name a live item in the workspace, else 400 validation_error, identical for nothing and for another workspace's item. - A personal token's workspace_id: a token could be pinned to a workspace the caller is not in. It must now be a live workspace they belong to, else the workspace 404 (also replacing a 500 for a nonexistent one). Each test fails on main and with its own fix reverted. Co-authored-by: Claude <noreply@anthropic.com> |
||
|
xarmian
|
7ee0bddc45 |
fix(webhooks): whole-workspace access to manage; no delivery for a deleted workspace (#1727)
* fix(webhooks): whole-workspace access to manage them; none for a deleted workspace (BUG-3340) A webhook receives item snapshots from every collection, but creating or test-firing one required only the owner role, so an owner restricted to specific collections could route every collection's content to a URL of their choosing. Create and test now require unrestricted workspace access, through the same check full export uses (BUG-1922), factored into requireUnrestrictedAccess. Delete stays open: it is revocation. ListWebhooks now returns nothing for a soft-deleted workspace. Both dispatcher delivery paths (Dispatch and DeliverEvent) list through it, so a deleted workspace's events are dropped instead of delivered during the restore window; a restore brings the webhooks back. Co-Authored-By: Claude <noreply@anthropic.com> * fix(webhooks): re-check the workspace before every send; hide URLs from restricted owners (BUG-3340) Codex r1 (no P0): - P1: ListWebhooks checked deletion once, but a delivery reuses the hook list and payload across inline retries, later hooks in the fan-out, and redirect hops; codex reproduced POSTs after the delete committed. The dispatcher store gains WorkspaceLive, checked before every attempt (a new deliverySuppressed result: nothing sent, not the endpoint's failure, not counted as matched) and before every redirect hop (the request carries the hook's workspace). A lookup error is transient, so a blip delays rather than drops or sends unchecked. - P2: the list still showed restricted owners every destination URL, and a URL can be the credential to a receiver's stored request bodies. A restricted owner now sees each webhook without its URL (the id is kept, so they can still delete it). Tests: retry, later hook and redirect after a mid-delivery deletion each send nothing (all three red with the checks disabled); the list hides URLs from the restricted owner only. Co-Authored-By: Claude <noreply@anthropic.com> * fix(webhooks): check the workspace on every connection; defer on lookup errors (BUG-3340) Codex r2 (no P0): - P1: net/http can send more than once inside one Do (it re-dials and replays the body after a reused connection fails with nothing written), and a deletion can land during dial or TLS setup, both after the per-attempt check. An httptrace GotConn hook now re-checks the workspace on every connection the request is about to be written on, after TLS and before writing, and cancels the request if it is gone. - P2 (two): a liveness lookup ERROR was treated as a deletion at the redirect check (the outbox then acked an undelivered event) and as an endpoint failure before a send (charged to the hook, which could auto-deactivate it). A new deliveryDeferred result sends nothing, keeps the event owed (Transient, so the outbox does not ack), and charges nothing to the hook. Suppression is reserved for a confirmed deletion. Tests: deletion before the connection is ready sends nothing; a pre-send lookup error and a redirect lookup error each defer without sending or charging. Each goes red when its own fix is reverted. Co-Authored-By: Claude <noreply@anthropic.com> * fix(webhooks): close the connection in GotConn, a real write barrier (BUG-3340) Codex r3 (no P0/P2): cancelling the request context in GotConn is not a write barrier for HTTP/1: the transport queues the request before it checks the context, so its writer could still send the whole request (codex measured 10/10,000 in the zero-send test). GotConn now also closes the connection it was handed when the workspace is gone or its liveness unknown: nothing can be written to a closed socket, and the cancel stops any transport retry on a fresh one. Measured here: the cancel-only version fails the zero-send test 2/10,000; with the close, 0/10,000 for both the deletion and lookup-error tests, and the webhooks package passes under -race. Co-Authored-By: Claude <noreply@anthropic.com> * fix(webhooks): the connection check decides the outcome even if Do returns a response (BUG-3340) Codex r4 (no P0/P1) P2: after GotConn closes and cancels, net/http can still return an already-buffered response, and the suppressed/deferred flags were only consulted when Do returned an error, so such a response counted as a delivery. The flags are now checked right after Do, whatever it returned, closing any response body first. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
xarmian
|
daf94614ca |
fix(documents): restore only a document of the URL's workspace (BUG-3335) (#1724)
handleRestoreDocument called store.RestoreDocument(id), which read and un-archived the document by ID alone and committed; only afterwards did the handler compare workspaces, and then "compensated" with a separate DeleteDocument. An editor of one workspace could restore another workspace's archived document by ID, exposing it until the re-delete and leaving its status, updated_at and attachment-reference stamps changed. RestoreDocument now takes the workspace and scopes both its SELECT and UPDATE by workspace_id, so a foreign document is sql.ErrNoRows and is never written; the handler answers 404 for that, 500 for any other error, and no longer compensates. Co-authored-by: Claude <noreply@anthropic.com> |
||
|
xarmian
|
6e30a549a1 |
fix(share-links): a link stops resolving while its workspace is deleted (#1726)
* fix(share-links): a link stops resolving when its workspace is deleted (BUG-3339)
DeleteWorkspace only sets deleted_at (the purge runs after the 30-day
restore window), and GetShareLinkByToken never checked the workspace, so
/s/{token} and /s/{token}/attachments/{id} kept serving a deleted
workspace's content for the whole window. The token lookup now joins
live workspaces, so such a link answers exactly like an unknown token;
restoring the workspace brings it back, since the link was never
destroyed. Both public share routes resolve the token through this one
lookup.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(share-links): the share resolver is never cacheable (BUG-3339)
Codex r1 (no P0/P1) P2: handleResolveShareLink emitted no Cache-Control,
so a heuristic cache could keep serving a deleted workspace's content
without reaching the token lookup, and a cached 404 could outlive a
restore. It now sets private, no-store on every answer, as the
attachment route already does; the test asserts it on 200 and 404.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
xarmian
|
3b27a138e4 |
fix(grants): an item grant never opens a deleted collection (BUG-3333) (#1725)
VisibleCollectionIDsQ promotes the collection of an item a restricted member is granted into their nav-only visible list, without checking the collection is live; GuestVisibleResourcesQ drops grants in deleted collections. After the collection was soft-deleted the member's grant list came back empty, the item handlers fell back to the visible list, and served every live item in the deleted collection (/items, /items-index, /items-changes, /tags, dashboard, graph, checkItemVisible). That promotion now requires a live collection. Assigned collections and whole-collection grants are unchanged: they already gave the whole collection, and they keep a grant on a deleted collection revocable. The delta-sync variant, GuestVisibleResourcesIncludeDeletedQ, also kept grants on deleted collections, so a guest still holding another live grant received the deleted collection's items as live upserts. It now keeps live collections only, like the live path; clients learn the collection went away from access_epoch (IDEA-2898). Co-authored-by: Claude <noreply@anthropic.com> |
||
|
xarmian
|
38c591673f |
fix(search): scope workspace search to visible items (#1723)
* fix(search): scope workspace search to visible items (BUG-3331) /api/v1/search is mounted outside RequireWorkspaceAccess and decides access itself. A caller with no visibility in a named workspace reached the store with an empty permission filter, which the store read as no filter at all. Store: the permission filter is fail-closed. SearchParams gains an explicit Unrestricted flag; empty CollectionIDs and ItemIDs (nil or not) now match nothing unless it is set. One helper applies the predicate on every query path (ref lookup, number lookup, FTS, count, facets) and adds a match-nothing predicate when a restricted filter is empty. Handler: searchAccessFor resolves each workspace the way RequireWorkspaceAccess does (fresh-install window, user-less workspace token limited to its own workspace, consent allow-list, cookie-only admin bypass, full and restricted members, grants for non-members), for both a named workspace and the unscoped fan-out. No access is an empty result, not a 403. Collections: a caller who sees a collection only through an item grant gets item_count and active_item_count computed from the granted items, not the whole collection. Co-Authored-By: Claude <noreply@anthropic.com> * test(search): cover the restricted number lookup, total, mixed fan-out and active count (BUG-3331) Codex r1 (gpt-6-luna, max scrutiny) found no P0/P1. Its P2 (a restricted member's direct grant to a soft-deleted collection now hides that collection's items from search while the item list still shows them) is search being the stricter of two doors; the item list's side is BUG-3333. Its test gaps are closed here: the bare-number lookup and the filtered total for an item-grant guest, a fan-out over workspaces with different access, and the active count following a closed granted item. Also: the named-workspace consent check is visible in handleSearch again (the allow-list coverage guard scans the handler; searchAccessFor still re-checks), and the now-unused removeString is gone. Co-Authored-By: Claude <noreply@anthropic.com> * fix(search): check the consent allow-list first in searchAccessFor (BUG-3331) Codex r2 (no P0/P1): the allow-list check came after the no-user branch, the reverse of RequireWorkspaceAccess, so a context with a mismatching allow-list and a matching legacy workspace token would have read its workspace. No current auth flow produces that combination, but the order now matches the middleware and the coverage claim is exact. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
xarmian
|
6f6e7ba176 |
chore(nix): bump package version to 0.18.0 ahead of the v0.18.0-rc.1 tag (#1721)
Claude-Session: https://claude.ai/code/session_01J9famMX8HvArgW2uo8mPRTv0.18.0-rc.1 |
||
|
xarmian
|
82064526e3 |
fix(brand): name the entity as Perpetual Software LLC everywhere (TASK-3321) (#1722)
Dave's ruling: the legal entity is Perpetual Software LLC and there is no dba, so "Perpetual Software" never names the entity without "LLC". Fixed: the ChatGPT plugin's author and developerName, both Claude plugin manifests' author/owner, the brand spec's copyright format, the cloud email footers (HTML and plain), and the auth-page footer. The guard test now pins the ChatGPT package's developerName and author to the full name, and the email tests expect it. LICENSE already said "Perpetual Software LLC"; goreleaser, the Dockerfiles and Nix carry no entity name. The self-hosted email test keeps asserting the bare substring is ABSENT: the shorter string is the stricter check there. Co-authored-by: Claude <noreply@anthropic.com> |