mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-24 03:16:43 +00:00
bfa32dde5ab00287eaeb25499fe75eedb5d297bf
339 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 xarmian
|
bfa32dde5a |
fix(security): encrypt webhook HMAC secrets at rest, mask in responses (BUG-2057) (#915)
Webhook signing secrets were stored plaintext in the webhooks.secret column and echoed back in every API response. Encrypt them at rest (reusing the existing AES-256-GCM store helpers, same pattern as TOTP secrets) and return the raw secret ONLY in the creation response; list responses now mask it and expose a has_secret flag instead. - store: encrypt on CreateWebhook, decrypt on Get/ListWebhooks so the dispatcher still signs with the plaintext secret. Reuses the secret column with the "enc:" prefix — no new column/migration. Keyless self-host stays a no-op fallback (encrypt returns plaintext; decrypt passes legacy rows through unchanged). - BackfillEncryptWebhookSecrets encrypts pre-existing plaintext rows on startup once a key is configured (idempotent), mirroring the TOTP backfill. - model: add HasSecret so masked responses still signal presence. - handlers: mask secret on list; document raw-only-on-create. - tests: encrypt-at-rest round-trip + HMAC validity, list decrypt, plaintext backfill/back-compat, and the API mask-except-on-create contract. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF |
||
|
xarmian
|
3bb50ab27f |
fix(security): make TOTP login codes single-use (BUG-2054) (#914)
The 2FA verify path accepted a valid TOTP code with no consumed-step tracking, so within a code's ~30s window (plus skew) the same code was replayable, and unlike the recovery-code branch the TOTP branch had no per-challenge attempt cap. Add a nullable users.totp_last_step column and an atomic compare-and-set Store.ConsumeTOTPStep: a code's derived time-step must be strictly greater than the stored watermark, and the winning UPDATE advances it in the same statement so two concurrent requests can't both consume one step. The handler derives the exact step a code matched (pinned within the ±1 skew window, not the current step) and rejects a replay with the same invalid-code response — no replay signal is leaked. Also caps TOTP attempts per challenge token by reusing the existing RecoveryCode limiter. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF |
||
|
xarmian
|
8f7b7d551f |
fix(security): rate-limit share-link password verification (TASK-2055) (#913)
Share-link password verification had no dedicated brute-force limiter, so a
password-protected /s/{token} link could be ground offline-fast — the resolve
handler would bcrypt-compare an unbounded stream of guesses.
Add two limiters, both charged BEFORE the bcrypt compare:
- SharePasswordIP (5 / 10-per-hour, keyed on SHA-256(share ID)+client IP)
caps a single grinder and protects bcrypt CPU; per-IP so one caller can't
lock out other viewers, and it's checked first so a single address can't
drain the link-wide bucket.
- SharePasswordShare (60 / 60-per-hour, keyed on SHA-256(share ID)) caps the
aggregate guess rate across a botnet that rotates IPs. Charged pre-compare
like login's per-email AuthEmail gate, so an exhausted link blocks even a
would-be-correct guess (no password oracle). Its burst is sized so ordinary
multi-viewer traffic never trips it, and the per-IP gate ahead of it means
exhausting it needs a genuine botnet (self-healing) — the same bounded
tradeoff AuthEmail accepts for an unauthenticated shared secret.
Both keyed on SHA-256 so no secret hits the limiter map.
Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF
|
||
|
xarmian
|
3f69b76b06 |
feat(security): enforce session UA binding under strict mode (TASK-2056) (#912)
Session IP/User-Agent binding was log-only by default, so a stolen session token granted durable any-origin access. IP-change enforcement already existed behind PAD_IP_CHANGE_ENFORCE=strict; this extends the same single toggle to also enforce the User-Agent-hash binding. When strict enforce is ON, a request whose client IP OR User-Agent hash no longer matches the session's stored binding now revokes the session (DeleteSessionIfExists) and rejects the request (401 for API, revoked-passthrough for public/browser paths), killing the stolen token. When enforce is OFF (default), behavior is unchanged: UA mismatch is logged (slog only, no new audit row) and the request proceeds, so existing self-host users see no behavior change and routine client churn (browser/WebView updates, DevTools emulation, mobile-app rebuilds) is tolerated. The UA hash is stable within a real session, so UA-mismatch enforce carries fewer false positives than IP enforce (mobile roaming, VPN toggles, carrier NAT) — documented in the handler comment. Adds the ActionSessionUAChanged audit action, emitted only in strict mode. No DB migration: reuses the existing IPChangeEnforce config flag and the existing session store primitives. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF |
||
|
xarmian
|
bed933d7fd |
feat(items): field-level PATCH + conflict envelope + read-only version history (TASK-2022) (#876)
* feat(items): field-level PATCH + conflict envelope + version history Adds three related item-update primitives (TASK-2022 / IDEA-1480): - Field-level merge: PATCH `fields_patch` shallow-merges onto the item's current fields INSIDE the write transaction (null deletes a key), so concurrent single-field updates no longer clobber each other via the full-blob read-modify-write. `pad item update` and the MCP `pad_item.update` action now send only the changed keys. - Optimistic concurrency: optional `expected_updated_at` on update; on mismatch the store returns *UpdateConflictError and the handler emits the pad-structured-error/v1 conflict envelope (HTTP 409, code=update_conflict). Surfaced on CLI (`--expected-updated-at`) and MCP (`expected_updated_at`). - Read-only version history: `pad item history <ref>` (alias `versions`) and MCP `pad_item.history`, reusing the existing item_versions store + versions endpoint (no new store, no schema change). MCP ToolSurfaceVersion bumped 0.9 -> 1.0 (new action + param; update behavior change). No migration required. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(items): address Codex review — dispatcher fields_patch, OCC ordering, date/required guards Round 1+2 review fixes for TASK-2022: - HTTP MCP dispatcher (dispatch_http_advanced.go) now sends fields_patch (only changed keys) instead of a client-side merged full fields blob, and forwards expected_updated_at — remote MCP callers get the same race-free merge + optimistic concurrency the CLI/HTTP paths do. - ValidatePartialFields rejects null-deleting a schema-declared REQUIRED field (would otherwise persist a blob the full-update validator rejects). - Open-children guard on the fields_patch path merges the patch onto the IN-TX locked row inside the precheck (not a stale pre-lock preview), so a priority-only patch can't false-fire the guard. - Optimistic-concurrency check now runs BEFORE the open-children precheck in the store, so a stale expected_updated_at yields update_conflict (not open_children) — single in-tx re-read shared by both. - Date auto-population on the patch path only fills an EMPTY current date; an existing end_date the caller isn't touching is preserved. Tests added for each fix. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS |
||
|
xarmian
|
c846cff4fd |
feat(project): agent-accessible activity feed (pad project activity + MCP action) (#877)
* feat(project): agent-accessible activity feed (pad project activity + MCP action)
Add a non-streaming, bounded activity query so agents can catch up on
what other agents/users changed since they last worked — the query
counterpart to the live `pad project watch` SSE stream.
- CLI: `pad project activity [--limit N] [--actor user|agent] [--since DATE]`
backed by the existing GET /workspaces/{ws}/activity feed.
- MCP: `pad_project.activity` action (passThrough) + cloud HTTP route.
- Extend the activity endpoint with a server-side `since` date filter
(handler parse + store SQL clause) so limit/actor/since behave
identically across CLI, stdio MCP, and cloud HTTP transports.
- Bump ToolSurfaceVersion 0.11 -> 0.12 (drift guard, README, CLAUDE.md,
instructions.md) and add a SKILL.md querying-guidance line.
Tests: store since-filter test, HTTP dispatch test, catalog action test.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
* fix(mcp): mark pad_project.activity read-only in tool surface
Add activity to readOnlyActions so the serialized MCP tool surface emits
read_only:true (missing entries default to write). Spot-check it in
tool_surface_test.go to guard against regression.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
|
||
|
xarmian
|
c127a5f965 |
fix(playbooks): enforce draft gate server-side + expose status (BUG-2020) (#874)
* fix(playbooks): enforce draft gate server-side + expose status (BUG-2020)
pad_playbook run / POST /playbooks/{ref}/run now refuse a playbook whose
status isn't "active" with a structured playbook_not_active error. Adds
an allow_draft escape hatch across all surfaces: JSON body field, CLI
--allow-draft flag, MCP boolean param, and an "allow-draft" bareword in
raw_args (stripped before strict parsing). status is now echoed on both
the run and get responses.
Bumps ToolSurfaceVersion 0.9 -> 0.10 and updates the drift-guarded docs
(instructions.md, README.md) plus CLAUDE.md.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
* fix(playbooks): forward allow_draft on WebMCP + refresh mcp serve help
Codex review follow-up for BUG-2020:
- WebMCP dispatcher + api client now forward allow_draft so the browser
surface can use the draft-gate escape hatch the catalog advertises.
- `pad mcp serve --help` refreshed from the stale "v0.4 / eight tools"
text to the current v0.10 / nine-tool surface (incl. pad_library).
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
* fix(playbooks): type playbooks.get() with top-level status (BUG-2020)
Codex P3 follow-up: the get response now returns Item & { status }.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
|
||
|
xarmian
|
ff7e7d51cb |
fix(server): guard degraded/degraded_sections in bootstrap dashboard (BUG-2072) (#869)
BUG-2014 (PR #867) added Degraded + DegradedSections to DashboardResponse so callers can tell a failed sub-query apart from a genuinely-empty section. BUG-2072 reported that the slim BootstrapDashboard projection omits them — but BootstrapDashboard embeds *DashboardResponse anonymously, so encoding/json already promotes both fields into the bootstrap wire shape. Verified empirically: the MCP pad_meta.action=bootstrap tool, the pad://workspace/{ws}/bootstrap resource, and the pad_set_workspace embed all serialize this same struct, so partial-failure state already reaches every agent surface. The promotion was untested and undocumented, so a future refactor to an explicit slim projection (like BootstrapCollection / BootstrapRole) could silently drop it. This pins the behavior: - TestBootstrapDashboardCarriesDegraded asserts on the marshaled JSON (not just promoted field access) that degraded=true + the failed section names flow through, and that a healthy dashboard omits degraded_sections. - BootstrapDashboard godoc now documents the promotion + the carry-across requirement for any future explicit projection. No payload change — the fields were already present. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS |
||
|
xarmian
|
a7994fc374 |
fix(store): make item field + parent-link update atomic (BUG-2013) (#868)
* fix(store): make item field + parent-link update atomic (BUG-2013) handleUpdateItem committed the field write, then ran SetParentLink/ ClearParentLink as a SEPARATE store transaction. A failure there (cycle discovered late, DB error) returned 500 with half the patch already applied — a code-acknowledged partial-commit window. Fold the parent-link mutation into the SAME transaction as the field write: - store: extract setParentLinkTx / clearParentLinkTx from the public SetParentLink / ClearParentLink (which keep their own tx). Add UpdateItemWithParentLink(id, input, precheck, *ParentLinkUpdate) — UpdateItemWithPreCheck now delegates to it with a nil link. The link write runs after the field UPDATE but before COMMIT, so a failing link write rolls the field write back too. - lock ordering: the NEW parent's advisory key is folded into the update's initial sorted AcquireParentChildrenLocks batch (extraKeys on acquireParentChildrenLocksForUpdate), so setParentLinkTx's later re-lock is an idempotent no-op and the combined update stays deadlock-free. checkParentCycle is parameterized over the queryer so the cycle walk reads inside the tx. - handler: restructured into validate / atomic-write / post-commit stages. The parent-link directive is built once and threaded through all three UpdateItemWithParentLink call sites; the post-commit SetParentLink/ClearParentLink block is removed. Works on both SQLite and Postgres (advisory locks are pg-only; SQLite gets atomicity from BEGIN IMMEDIATE). Adds store tests proving a failing parent-link write rolls back the field change (no partial state) and that the happy path commits both together. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(store): check parent cycle under lock in setParentLinkTx (Codex #868) Codex review flagged a cycle-check TOCTOU: setParentLinkTx walked the ancestor chain BEFORE acquiring the parent-children advisory locks, so two concurrent reparents could each pass on a stale snapshot, block on the lock, then both insert — forming a cycle (A→B→C→A). Move the cycle check to AFTER lock acquisition; under the lock the tx-scoped walk sees the edge the just-unblocked peer committed and rejects the cycle. Pre-existing behavior (the old SetParentLink checked cycles on s.db before even beginning its tx), hardened here since this function was already being refactored. Residual: cycles closed via an edge on an item neither endpoint locks remain possible — a limitation of the per-endpoint lock scheme, tracked separately. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS |
||
|
xarmian
|
aeb80883f0 |
fix(webhooks): track delivery goroutines + bounded retry (BUG-2012) (#864)
* fix(webhooks): track delivery goroutines + bounded retry (BUG-2012) Webhook deliveries ran in untracked `go d.deliver(...)` goroutines that write to the store — the BUG-842 shutdown-race class that goAsync was built to prevent — and had no retry. - Inject a `spawn func(func())` into Dispatcher (SetSpawn). Server wires s.goAsync via SetWebhookDispatcher so deliveries are tracked on s.bg (Stop() waits for in-flight deliveries) and inherit goAsync's panic recovery (BUG-2011). Nil spawn falls back to a plain goroutine, so standalone Dispatcher usage is unchanged. - Add a bounded in-goroutine retry: up to 3 attempts with linear backoff on transient failures (network error / timeout / 5xx). Permanent failures (4xx, SSRF block, malformed URL) stop immediately. The final outcome is recorded once via UpdateWebhookFailure. - Tests: delivery runs on the injected spawn; transient 5xx retries to the cap; permanent 4xx does not; a recovered transient records success. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS * fix(webhooks): classify redirect-block + non-5xx as permanent (Codex review) Second Codex review of PR #864 found two retry-classification gaps: - P2: an SSRF-blocked (or looping) redirect surfaces as an error from client.Do (via CheckRedirect), which the retry loop treated as transient — so a redirect to an internal target was retried 3x with backoff. Wrap a sentinel (errRedirectRejected) in checkRedirect and match it with errors.Is (url.Error unwraps to it) to classify these as permanent — attempted once, no retries. - P3: the status switch treated every non-2xx/non-4xx as transient. Narrow transient to 5xx only; 4xx/3xx-no-Location/1xx are permanent, matching the stated "network error / timeout / 5xx" retry policy. Adds TestDispatcher_RedirectBlockIsPermanent. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS |
||
|
xarmian
|
8be1ac67da |
fix(server): surface dashboard sub-query failures instead of silent empty sections (BUG-2014) (#867)
* fix(server): surface dashboard sub-query failures instead of silent empty (BUG-2014)
buildDashboardResponse assembled several best-effort sections with
`if err == nil` / `if err != nil { continue }` and no logging, so a
failing sub-query rendered indistinguishably from a genuinely-empty
section — a completely silent degradation.
Add a `Degraded` bool + `DegradedSections []string` to DashboardResponse.
A new markDegraded helper logs each failure (slog.Error with workspace +
section) and records the affected section, so partial failures are both
diagnosable server-side and visible to the client without changing the
all-or-nothing contract for the queries whose failure genuinely
invalidates the whole dashboard (those still return an error). Wired into
active_plans, attention.stalled, attention.orphaned_tasks, recent_activity,
by_role, and starred_items. The has_agent_activity source fallback (which
has a valid default) logs a Warn but does not degrade.
Mirror the new fields in the TypeScript DashboardResponse type and add a
Go test asserting a failed sub-query flips Degraded, names the section,
keeps the endpoint at 200, and preserves the healthy sections.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
* fix(server): skip orphan detection when GetParentMap fails (BUG-2014 review)
A GetParentMap failure previously fell back to an empty parent map and
still iterated allTasks, flagging every visible non-done task as an
orphaned_task (false positives). Skip orphan detection entirely on that
failure — the section is already marked degraded.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
* feat(web): show degraded-load banner on the dashboard (BUG-2014)
Consume the new DashboardResponse.degraded / degraded_sections signal on
the workspace dashboard page. When a best-effort sub-query fails
server-side, the affected sections could otherwise render as genuinely
empty; surface an amber "some data couldn't be loaded" banner (listing
the affected sections) so the partial-failure state is visible instead of
silent.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
|
||
|
xarmian
|
a04fd861dc |
fix(server): add panic recovery to background sweeper goroutines (BUG-2071) (#865)
The four long-running sweeper loops (orphan GC, op-log GC, token reaper, workspace purge) spawn their own s.bg-tracked goroutine with a stop-channel lifecycle, so they can't route through goAsync (a fire-and-forget helper that owns the whole goroutine) without breaking shutdown or double-counting s.bg. As a result they had NO recover(): a panic in any sweeper body crashed the single-binary server for every tenant. Add a shared Server.recoverSweeper(name) firewall — mirroring goAsync's recover + debug.Stack slog style — and defer it inside each sweeper goroutine. A panic is now logged with a stack and the goroutine unwinds cleanly; its own deferred s.bg.Done() still fires (recover stops the unwind), so Stop() still drains. No change to any sweeper's loop cadence or stop-signal shutdown. Adds TestTokenReaper_RecoversPanic, which drives a real reaper tick to panic (nil store → nil-pointer deref in the first cleaner) and asserts the panic is logged+recovered and Stop() returns. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS |
||
|
xarmian
|
f7c4cb3287 |
fix(server): add panic recovery to goAsync background tasks (#863)
goAsync wrapped fn in a bare goroutine with no recover(); chi's Recoverer only covers request goroutines, not these detached ones. A panic in a background task (e.g. deriveThumbnails hitting a Go image-decoder panic on a crafted upload, or an email send) would unwind past the goroutine and crash the whole single-binary server for every tenant. Add a single deferred recover() inside the goAsync goroutine that logs the panic + stack via slog, covering all 15+ call sites at once. The recover defer is registered after `defer s.bg.Done()`, so it runs first on unwind and Done() still fires — Stop() continues to drain the WaitGroup even when fn panics. Adds TestServer_goAsync_RecoversPanic asserting the process survives a panicking fn and Stop() returns. Fixes BUG-2011. Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS |
||
|
xarmian
|
15ad930d78 | feat(bootstrap): add convention_index for triggered-convention discovery (TASK-2004) (#848) | ||
|
xarmian
|
48104a5eff |
perf(server): collapse dashboard/bootstrap N+1 into set-based queries (BUG-2002) (#847)
The dashboard builder (also reused by the bootstrap endpoint and every pad_set_workspace) ran ~1000 queries on a large workspace: one GetItemLinks + per-link GetItem for every non-done item (blocked attention + suggested_next filter), a GetChildItems per active plan (progress + suggested_next), a GetItemIncludeDeleted per recent-activity row, a GetCollection per visible collection, and a per-collection COUNT via ListCollections whose result the dashboard never uses. Replace every per-item/per-row loop with a set-based query: - GetBlocksEdges: one workspace-wide JOIN of blocks-links -> blocker essentials, ordered created_at DESC to preserve the old first-active- blocker selection. Drives both blocked attention and the suggested_next blocked-filter (retires itemBlockedByActive). - GetChildItemsForParents: one IN query grouping all active-plan children by parent (progress + suggested_next; no-content projection). - GetItemsByIDsIncludeDeleted: one IN query batch-hydrating recent-activity items (include-deleted). - ListItemsParams.NoContent: skip loading full markdown bodies on the count/summary scans (allItems, plans, stalled, orphaned). - ListCollectionsMinimal now also selects slug; the dashboard uses it in place of ListCollections, dropping the unused per-collection COUNT N+1 and the GetCollection-per-visible-id loop. Per-item N+1s are gone; query count is now constant in workspace size. Verified byte-identical dashboard + bootstrap JSON against three live workspaces (docapp/claude/apm); dashboard latency ~376ms -> ~198ms on the 1907-item docapp workspace. New store methods are unit-tested. |
||
|
xarmian
|
375e3b5369 |
perf(store): batch parent-lineage enrichment into one scoped query (BUG-2003) (#846)
enrichItemsWithParent loaded every parent link in the workspace and then called full-row GetItem once per unique parent (151 in the live workspace), despite a comment claiming a bulk fetch. A ?limit=1 list took 34-39ms vs ~2ms for a single GET — a ~20x tax to return one row, hit on every list request including the /items-changes sync endpoint the local-first client polls. Scope the parent IDs to only the parents of the returned item slice, then hydrate title/ref/slug/collection in one skinny WHERE id IN (...) query via the new Store.GetItemLineageByIDs. Enrichment output shape and best-effort (missing parent never fails the list) behavior are preserved; the visibility filter now runs against the batched projection's collection_id. |
||
|
xarmian
|
0aa431f132 |
fix(server,cli,mcp): default item list to per-collection non-terminal filter (BUG-2001) (#845)
The CLI's default `pad item list` (no --status/--all) sent a hardcoded ~20-status allowlist as the status filter. Collections with custom status vocabularies (blog: drafting/scheduled; human-tasks: todo) fell outside the list and had their open items hidden. MCP inherited the same bug via the CLI default and the HTTP route table's mirrored allowlist. Replace it with a server-side `non_terminal` filter: ItemListParams.NonTerminal resolves each collection's terminal set from its schema's terminal_options (falling back to DefaultTerminalStatuses) and keeps only items NOT in that set — reusing the existing doneFiltersForWorkspace + buildChildrenDoneExpr machinery, applied in both the normal and FTS query paths. The CLI default and both MCP dispatch paths (ExecDispatcher via the CLI, HTTPHandlerDispatcher via mapItemList) now send non_terminal=true. --status X and --all semantics are unchanged. |
||
|
xarmian
|
cf5eb8dd3a |
feat(cli,mcp): summary-shaped item list with --full opt-in + limit clamp (TASK-2000) (#842)
`pad item list --format json` returned the full models.Item shape — including each item's rich markdown `content` body (~52% of the bytes) plus UUID plumbing and duplicate join fields — with no default limit, so a bare agent list dumped ~1.4MB (all collections) or 5.3MB (--all) into context. The single biggest agent-token lever. CLI: - JSON output now defaults to a token-light ItemSummary projection: `content` → short `content_preview`, UUIDs (id/workspace_id/collection_id/*_user_id/ parent_id/agent_role_id) and duplicate collection/parent join fields dropped, `fields`/`tags` emitted as nested JSON. ~71% smaller on a real workspace. - `--full` opt-in flag restores the complete models.Item shape. - Default limit (200) + hard-max clamp (1000) so --all/huge lists can't dump unboundedly; a stderr note fires when a table result is capped. MCP: - pad_item.list is now a custom action that injects a default limit (50) and clamps an oversized one (max 300), mirroring the backlinks default/max, so a bare agent list stays bounded on both dispatchers. - ToolSurfaceVersion 0.8 → 0.9 (list result shape + limit behavior change). Server: - Hard-max backstop clamp (1000) on an explicit `?limit=` at the item-list request boundary; no default (internal ListItems callers that fetch every row are untouched). rawJSONOrNil guards against a malformed stored Fields/Tags value breaking the whole list marshal (falls back to a JSON string). |
||
|
xarmian
|
f5b437a65f |
feat(server): workspace restore + deleted-list endpoints (TASK-1970) (#827)
Foundation for PLAN-1969 (user-recoverable workspace soft-delete). A
workspace delete only stamps workspaces.deleted_at; items/collections/
members are untouched, hidden transitively. Restore clears deleted_at so
everything re-surfaces intact.
Store (internal/store/workspaces.go):
- RestoreWorkspace(slug): UPDATE ... SET deleted_at = NULL WHERE slug=?
AND deleted_at IS NOT NULL. Returns sql.ErrNoRows (-> 404) when no
soft-deleted row matched (already live or purged).
- ListDeletedWorkspaces(userID, cutoff): owner-scoped, deleted_at within
the window, ordered deleted_at DESC. Account-deleted workspaces have no
live owner, so they never leak.
- GetDeletedWorkspaceBySlug(slug): resolves a soft-deleted row (the normal
resolvers filter deleted_at IS NULL) so the handler can tell 403 from 404.
- Dual-dialect via s.q/s.dialect; no migration (deleted_at already exists).
Handlers (internal/server/handlers_workspaces.go):
- POST /api/v1/workspaces/{slug}/restore: owner-only; 404 not-restorable,
403 non-owner, 200 + restored workspace; logs a "restored" activity.
- GET /api/v1/workspaces/deleted: owner-scoped list with per-entry
purge_at + days_left, both derived from workspacePurgeRetention so
restore and the purge sweeper share ONE 30-day window (no drift).
- Both routed outside the /{slug} RequireWorkspaceAccess subrouter (which
resolves only live workspaces); restore enforces owner authz inline.
CLI client (internal/cli/client.go): RestoreWorkspace + ListDeletedWorkspaces.
TS type (web/src/lib/types/index.ts): Workspace.deleted_at + DeletedWorkspace.
Tests: store (resurface-intact; double-restore/live -> ErrNoRows; window
boundary 29d IN / 31d OUT + owner-scoping) and handler (owner-only 403,
404 live/unknown, 200 restore, owner-scoped deleted-list). Green on
SQLite and Postgres (make test-pg); golangci-lint clean.
Closes TASK-1970
Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST
|
||
|
xarmian
|
b73ba63752 |
feat(store): hard-purge soft-deleted workspaces after 30 days (TASK-1966) (#825)
The /privacy policy promises owned workspaces are removed from live systems within 30 days, but DeleteAccountAtomic and DeleteWorkspace only SOFT-delete (workspaces.deleted_at) and nothing ever expunged them — a right-to-erasure gap. Add a scheduled sweeper that hard-purges workspaces soft-deleted longer than a named 30-day retention constant. - Store: ListPurgeableWorkspaces (soft-deleted + past cutoff; never touches live rows), WorkspaceAttachmentBlobs, CountAttachmentsForHash- OutsideWorkspace (content-addressed dedupe guard), and PurgeWorkspace- Data — a transactional cascade that deletes every workspace-scoped child row in FK-dependency order (items/comments/versions/links/ reactions/stars/yjs op-log/wiki-links/grants/transitions/moves/views/ collections/documents+versions/agent_roles/webhooks/invitations/ templates/share_links+views/oauth join rows/report layouts/members/ member access/api tokens/attachments/activities), de-identifies mcp_audit_log, and refuses to touch a non-soft-deleted workspace. - Server: a periodic sweeper modeled on the orphan GC — captures blob keys before the purge, cascades the DB rows, then reclaims blobs through the attachment store abstraction (FS + S3 safe) with the orphan GC's cross-workspace dedupe + in-flight-upload guards. Failure isolated per workspace; idempotent. - Dual-dialect (SQLite + Postgres); partial index on workspaces(deleted_at) — migrations/073 + pgmigrations/051. Both delete paths (account + manual workspace delete) purge on the same 30-day clock: identical deleted_at mechanism, both owner-initiated, and the orphan GC already reclaims their attachment blobs at 30 days. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
86a952768e |
test: cover account delete + export contracts and Danger Zone e2e (TASK-1963) (#824)
Backend contract tests (internal/server):
- delete-account success-body: pin the exact {ok:true} envelope the UI
consumes (the cascade/skip tests asserted only status 200).
- export happy-path: decode the artifact and assert the exact
`attachment; filename="pad-export.json"` header, application/json type,
and the top-level {user, workspaces} shape with inline collections/items
(fills the TASK-508 gap; complements the BUG-1945 gate smoke test).
- TOTP paths (enabled+valid/missing/invalid, non-TOTP) were already added
in TASK-1958 — confirmed, not duplicated.
Web e2e (web/e2e, Playwright): settings Danger Zone —
- export download (filename + success line),
- delete password branch (real register→login→delete; admin user search
confirms the row is gone),
- delete cloud OAuth-only typed-confirm branch (session/me flags patched;
delete transport stubbed since a self-host server requires a password),
- post-delete redirect to /login.
Delete specs run desktop-only (viewport-agnostic; avoids doubling
IP-rate-limited /auth/login + /auth/register hits that flaked the suite
under parallel load).
No product code changed.
Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST
|
||
|
xarmian
|
022280a1ee |
feat(server): require TOTP re-verification to delete account (#822)
handleDeleteAccount previously verified only the password (or, in cloud mode, a confirm-only session) and never re-checked TOTP even when the account had 2FA enabled. That made an irreversible account destroy a lower bar than login for 2FA users — a hijacked live session (cloud confirm-only needs no password) could wipe the account. Add an optional totp_code to the delete-account request body. When the authenticated user has TOTP enabled, require and verify the code server-side (via the existing totp.Validate path used by login and 2FA disable) AFTER the password/confirm identity check and BEFORE the Stripe cancel + local delete — so neither the password nor the cloud-confirm path can bypass it, and a failed code never leaks a cancel RPC. Missing code → 400 totp_required; wrong code → 401 totp_invalid. Users without TOTP are unaffected (no code required, same behavior as before). Tests cover TOTP-enabled + valid code (success), missing code (totp_required), invalid code (totp_invalid), and non-TOTP (unaffected), reusing the fakeSidecar + bootstrapAccountDeleteUser + deleteAccountReq harness. Closes TASK-1958 Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
366d4fb7e5 |
fix(store): harden account-deletion FK cascade (TASK-1959) (#821)
* fix(store): harden account-deletion FK cascade (TASK-1959)
DeleteAccountAtomic could 500 with nothing deleted when a user had rows
referencing them via foreign keys with no ON DELETE action — notably
activities.user_id (the audit/history log, including the session_ip_changed
rows the auth middleware writes on the very request that deletes the
account). The delete-account tests only passed by working around this
(pinning RemoteAddr to loopback, scrubbing activities.user_id).
Audit every table with a FK to users(id) and handle each in the delete
transaction:
- de-identify (UPDATE ... SET NULL) audit/history rows: items
created/modified, comments, comment_reactions, item_links,
item_versions, share_link_views
- delete owned/transient/audit rows: sessions, api_tokens,
workspace_members, sent invitations, password/email tokens, issued
grants, created share links, mcp_audit_log, oauth_connections
- rely on existing ON DELETE CASCADE / SET NULL for item_stars,
user_report_layouts, {collection,item}_grants.user_id,
items.assigned_user_id
Migrations 072 (SQLite) / 050 (Postgres) give activities.user_id an
ON DELETE SET NULL FK so the highest-write-frequency audit table can't
block a delete via a row written concurrently during the request. SQLite
rebuilds the table (022_audit_trail pattern); Postgres never had the FK,
so it is added after an orphan scrub so validation passes.
Remove the test work-arounds now that the cascade holds: deleteAccountReq
deletes from a changed IP so the session-IP-change audit row exercises the
fix, and the partial-delete test injects its post-cancel failure via the
sidecar hook instead of the (now-fixed) FK gap. Add a store-level test that
deletes a fully-populated user atomically.
Closes TASK-1959
Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST
* fix(store): harden account-deletion FK cascade (TASK-1959)
DeleteAccountAtomic could 500 with nothing deleted when a user had rows
referencing them via foreign keys with no ON DELETE action — notably
activities.user_id (the audit/history log, including the session_ip_changed
rows the auth middleware writes on the very request that deletes the
account). The delete-account tests only passed by working around this
(pinning RemoteAddr to loopback, scrubbing activities.user_id).
Audit every table with a FK to users(id) and handle each in the delete
transaction:
- de-identify (UPDATE ... SET NULL) audit/history rows: items
created/modified, comments, comment_reactions, item_links,
item_versions, share_link_views
- delete owned/transient/audit rows: sessions, api_tokens,
workspace_members, sent invitations, password/email tokens, issued
grants, created share links, mcp_audit_log, oauth_connections
- rely on existing ON DELETE CASCADE / SET NULL for item_stars,
user_report_layouts, {collection,item}_grants.user_id,
items.assigned_user_id
Migrations 072 (SQLite) / 050 (Postgres) give activities.user_id an
ON DELETE SET NULL FK so the highest-write-frequency audit table can't
block a delete via a row written concurrently during the request. SQLite
rebuilds the table (022_audit_trail pattern); Postgres never had the FK,
so it is added after an orphan scrub so validation passes.
Log the account_deleted audit row with an empty user_id (deleted id kept
in metadata): the user row is already gone by then, and the new
activities.user_id FK would otherwise reject the insert and silently drop
the row. This makes the account_deleted event actually recorded on both
dialects.
Remove the test work-arounds now that the cascade holds: deleteAccountReq
deletes from a changed IP so the session-IP-change audit row exercises the
fix, and the partial-delete test injects its post-cancel failure via the
sidecar hook instead of the (now-fixed) FK gap. Add a store-level test that
deletes a fully-populated user atomically.
Closes TASK-1959
Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST
|
||
|
xarmian
|
1d8b04e279 |
feat(server): expose password_set on /auth/me + TS User type (#819)
The delete-account UI must branch between a password prompt (self-host or any user with a password) and a confirm-only flow (OAuth-only users with no password). The client had no signal for this: oauth_providers is not a valid proxy since a user can have both a password and linked OAuth. Add "password_set": user.HasPassword() to the /auth/me response map and password_set?: boolean to the TS User interface. A handler test asserts the field for both a password user (true, via bootstrap) and an OAuth-only user (false, via CreateOAuthUser). Closes TASK-1957. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
e6eec608e0 |
feat(web): default agent-connect modal to MCP setup, steer zero-grant users (#817)
Reorder ConnectWorkspaceModal so MCP setup is the default and first tab — most users landing here have never connected an agent, so the OAuth "fresh agent" path is their real first step. CLI is second; the claim code moves to a third "Connect code" tab, reframed as a scoped-grant add-on rather than the (misleading) "recommended" default it was. Close the zero-grants dead end: add Store.HasActiveConnectionForUser and surface has_any_connection on the claim-code endpoint, so a user who opens the Connect-code tab with no connected agent gets steered to set one up first instead of a live-looking but unredeemable code. Hide the MCP + code tabs on self-host deployments without a public MCP URL (both depend on the remote OAuth server), leaving CLI as the sole, default path there. Verified: go build ./..., go test ./internal/server/ ./internal/store/, web npm run check (0 errors), and a Codex review all pass clean. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
7eff417d83 |
fix(server): deny restricted owners from escalating collection access (BUG-1925) (#815)
handleSetMemberCollectionAccess gated only on requireRole(r, "owner"), with no validation of the caller's own visibility. member_collection_access has no role exclusion (BUG-1920), so a workspace-role owner independently restricted via collection_access="specific" could PATCH themselves (or any other member) to mode="all", or grant a collection outside their own visible set — defeating the entire BUG-1917/1918/1920/1921 visibility family plus the BUG-1922 export gate in one authenticated request. requireCallerCanSetCollectionAccess now denies a restricted caller from setting mode="all" for any target, and validates every requested collection ID against the caller's own visible set, narrowed to guestResourceFilter's fullCollIDs (mirroring requireCollectionFullyVisible, BUG-1920 codex R2) so an item-level grant can't be escalated into collection-wide access. Unrestricted callers are unaffected. |
||
|
xarmian
|
84637ba6cb |
fix(server): deny account export to restricted owners (BUG-1945) (#814)
handleExportAccount (/auth/export) dumped full collections/items for every workspace a user owns without checking collection_access, bypassing BUG-1922's workspace-export gate via a different route. A restricted owner (collection_access="specific") could exfiltrate hidden collections through the account-export affordance instead. Mirror BUG-1922's outright-deny: before any response byte is written, scan every owned workspace with visibleCollectionIDs and refuse the whole export with 403 if any owned workspace is restricted, rather than silently omitting it from an unflagged partial dump. |
||
|
xarmian
|
b633144009 |
fix(server): deny workspace export to restricted owners (BUG-1922) (#813)
Workspace export (both the JSON and tar.gz bundle forms of GET
/workspaces/{slug}/export) streamed the full unfiltered workspace
regardless of the caller's collection_access, letting a restricted
owner exfiltrate collections hidden from them. Per Dave's ruling,
export is a backup/portability affordance rather than a
visibility-scoped view, so a restricted caller is denied outright
(403) instead of receiving a filtered subset.
|
||
|
xarmian
|
d36f27c29f |
fix(server): auth-perimeter hardening — B6–B9 from the IDEA-1927 audit (TASK-1932) (#811)
* fix(server): stop autoCreateWorkspace from swallowing member-add errors (B6, TASK-1932) A failed AddWorkspaceMember after workspace creation used to be silently discarded, leaving a workspace that's completely unreachable (owner_id alone grants no access) and invisible in the console forever. Retry once, then clean up the orphaned workspace and log loudly on continued failure so on-call can act on it. * fix(server): fail fast when cloud mode runs without secure cookies (B7, TASK-1932) SetCloudMode never forced secureCookies on, so PAD_CLOUD=true without PAD_SECURE_COOKIES was an unenforced ops contract: OAuth's __Host-prefixed session cookie is silently invisible to pad's own cookie reader without Secure set, producing a "logged in but appears logged out" failure mode. Add Config.ValidateCloudSecureCookies and check it at server startup, next to the existing PAD_CLOUD_SECRET requirement, so the misconfiguration is a startup error instead of a runtime mystery. * fix(server): align OAuth session TTL with web session TTL (B9, TASK-1932) handleOAuthLogin minted a 30-day session while every other web login used the 7-day webSessionTTL. createAuthSession derives the store session row, session cookie MaxAge, and CSRF cookie MaxAge all from one ttl argument, so the longer OAuth cookie outlived its own server-side session — the browser kept presenting a cookie whose session had already expired, producing silent 401s. Use webSessionTTL for OAuth logins too. * fix(server): narrow the /api/v1/auth/* CSRF exemption to anonymous endpoints (B8, TASK-1932) The CSRF middleware exempted the entire /api/v1/auth/ prefix, which also covered mutating cookie-authenticated endpoints: PATCH /me, oauth-unlink, 2FA setup/verify/disable, delete-account, token create/delete/rotate, CLI- session approve, and logout. Replace the prefix bypass with an exact-path allowlist of the endpoints that are genuinely pre-session (login, register, bootstrap, password reset, verify-email, resend-verification, 2FA login challenge, CLI session create) or authenticate purely via a cloud secret rather than a cookie (oauth-login, oauth-link — never touch the session, so CSRF isn't a meaningful threat model for them and the sidecar has no CSRF cookie to send). Everything else now requires the double-submit token like any other authenticated mutation; the web client already sends it on every non-GET/HEAD request, so no frontend change is needed. * docs(server): pin the deliberate CSRF-cookie legacy-fallback asymmetry (TASK-1932) Codex review (round 1) flagged that SessionAuth falls back from the __Host-pad_session cookie to the legacy unprefixed name, but the CSRF cookie lookup has no equivalent fallback — meaning a browser holding pre-secure-cookies-flip legacy cookies stays authenticated but gets 403'd on B8's newly CSRF-required endpoints until it re-logs-in. This asymmetry is deliberate, not a bug: the session cookie's value is an unguessable secret regardless of which name carries it, but the CSRF cookie's security property depends on the attacker being unable to set the cookie itself — an unprefixed name is settable from a sibling subdomain, which is exactly the hole __Host- exists to close. Restoring "symmetry" here would silently reopen it. Document the reasoning at the cookie lookup so a future maintainer doesn't "fix" it, and add a pinning test that exercises the exact scenario (secureCookies=true, legacy session + CSRF cookies, CSRF-required endpoint) end to end. * fix(server): require CSRF for session-authenticated requests to exempt auth paths (TASK-1932) Codex round 2 found a P1: handleRegister has an admin-session branch (an already-logged-in admin can create a verified account with no invitation code), but /api/v1/auth/register was unconditionally CSRF-exempt by path. A cross-site POST could ride the admin's cookie into that branch with no CSRF token — the same class of hole as the oauth-unlink case B8 already closed, just missed because register's other paths are genuinely anonymous. Fix generically rather than register-specifically: gate the authCSRFExemptPaths exemption on currentUser(r) == nil. SessionAuth runs before CSRFProtect, so a request that resolved to a real session falls through to the normal double-submit check instead of the early exemption, while a genuinely anonymous request keeps it. This also covers any future session-authenticated branch a handler on this list grows, with no handler changes. Bearer/PAT and cloud-secret (oauth-login/oauth-link) callers are unaffected — they have their own unconditional exemptions later in the same function. * fix(server): require validated Bearer/cloud-secret auth for CSRF exemption (TASK-1932) Codex round 3 found that CSRFProtect's Bearer and X-Cloud-Secret exemptions fired on header/marker PRESENCE, not validation. TokenAuth deliberately falls through (rejectInvalidBearer) instead of 401ing invalid Bearers on /api/v1/auth/* paths to support CLI-token recovery, so a cross-site request carrying a victim's real session cookie plus a garbage Bearer header could ride the cookie past CSRF on any newly-CSRF-required endpoint. The same presence-only pattern in the X-Cloud-Secret exemption is concretely exploitable too: handleSetPlan (and similarly-shaped handlers) accept an admin cookie session as an alternative to the secret, so a garbage X-Cloud-Secret plus a stolen admin cookie could set an arbitrary user's plan with no CSRF token at all. Add ctxValidatedSessionBearer (set by TokenAuth only on successful ValidateSession for CLI session-bearer tokens) alongside the existing ctxIsAPIToken, and a combined isValidatedBearerAuth() helper. CSRFProtect now exempts unconditionally only on validated Bearer auth; an unvalidated Bearer header or cloud-secret marker is exempt only when no session was also resolved for the request (currentUser(r) == nil), preserving the CLI-recovery contract (stale token, no cookie -> 401 from auth, not csrf_error) while closing the cookie-riding case. * fix(server): split CSRF auth-exempt allowlist by session sensitivity (TASK-1932) Codex round 2 gated the entire authCSRFExemptPaths allowlist on currentUser(r) == nil to close handleRegister's admin-session branch, but that gate applied to every anonymous endpoint on the list, not just register. CI's E2E suite caught the regression: the harness bootstraps an admin (minting a session cookie) then POSTs /login to re-authenticate, and the ambient cookie stripped /login of its exemption, producing a spurious 403 csrf_error. login/bootstrap/forgot-password/reset-password/local-reset/verify-email/ resend-verification/2fa-login-verify/oauth-login/oauth-link/cli-sessions- create derive their authority entirely from the request body (credentials, a token, a shared secret), never from the ambient cookie, and pad mints the CSRF cookie AT LOGIN — a pre-session endpoint categorically cannot require a token that doesn't exist yet. Split the allowlist: authCSRFUnconditionalExemptPaths (everything above, exempt regardless of cookie) and authCSRFSessionGatedExemptPaths (register only, exempt only when currentUser(r) == nil, since it alone has a session-privileged admin-account-creation branch). The round-2 security property (admin session + register + no CSRF -> still blocked) and round-3's Bearer/ cloud-secret validated-vs-present composite are unaffected. |
||
|
xarmian
|
f34e66254b |
feat(admin): admin force-verify email override (TASK-1939) (#809)
Wave 4 of PLAN-1933 (DR-7). Adds a web-console-only admin override to
force-verify a locked-out unverified account. No CLI, no MCP (matches
the no-auth-mutation-on-MCP rule).
Server:
- POST /api/v1/admin/users/{userID}/verify-email — admin-only, mirrors
handleAdminEnableUser. Reuses the existing SetUserEmailVerified store
method (added in Wave 3b) and audits with the distinct
ActionEmailVerifiedByAdmin action (separate from the self-serve
ActionEmailVerified — a force-verify is an operator security action).
Idempotent (already-verified returns 200 no-op).
- Surface email_verified_at in the admin list + get-user JSON so the
console knows verified state (Wave 1 only added the store-level scan).
Web:
- adminVerifyEmail client method (api.admin.verifyEmail) confined to the
admin section of client.ts.
- "Mark email verified" action in the admin user panel (UserSettingsForm),
shown only when the target user is unverified.
- email_verified_at added to the AdminUser type.
Tests: admin force-verifies an unverified user (flips email_verified_at +
audits ActionEmailVerifiedByAdmin, not the self-serve action) and the
now-verified session is unblocked; non-admin -> 403 with no side-effect.
Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST
|
||
|
xarmian
|
4a7c054223 |
feat(server): cloud email self-registration + verify-email/resend endpoints (TASK-1938) (#808)
Wave 3b of PLAN-1933 turns ON Pad Cloud email/password self-registration with mandatory email verification. - DR-6: relax handleRegister to allow self-serve signup when cloudMode && emailConfigured. emailConfigured = s.email != nil AND a USABLE public base URL (non-empty, not a 0.0.0.0/:: bind-all host), so no unverifiable user is ever created. Self-serve is the ONLY path that writes email_verified_at = NULL (UserCreate.Unverified); admin-created and invited signups stay verified. Mints + sends a verification email. - DR-5: POST /auth/verify-email (ConsumeEmailVerification → flips email_verified_at → returns fresh user) and POST /auth/resend-verification (always-200, enumeration-safe; minting a new token invalidates the prior one). Both wired into the rate-limiter path switch (PasswordReset bucket). - DR-1: handleAcceptInvitation verifies an unverified account on accept (email-bound invite proves email control), via new store method SetUserEmailVerified. - DR-11: keep the existing clear 409 on duplicate email at signup. Session freshness: currentUser is re-read fresh from the DB per request (ValidateSession → GetUser), so flipping email_verified_at unblocks the same session's subsequent mutations immediately under RequireVerifiedEmail (Wave 3a) — no session-row rewrite needed. Test covers verify → same-session mutation succeeds. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
31053086ee |
feat(server): RequireVerifiedEmail enforcement across all mutation perimeters (TASK-1937) (#807)
PLAN-1933 DR-4 (Wave 3a). Enforce, on Pad Cloud only, that an
authenticated-but-unverified user cannot mutate content or mint
credentials. Cloud-only + unauthenticated + verified are all no-ops, so
this is inert in production until Wave 3b starts creating unverified
users; the tests drive the state directly via the store's explicit
UserCreate.Unverified control.
Core rule: block only when cloudMode && currentUser != nil &&
!IsEmailVerified(), on mutating methods (POST/PATCH/PUT/DELETE). Returns
403 email_not_verified. The middleware does NOT inherit CSRFProtect's /
RequireAuth's blanket /api/v1/auth/* exemption — it decides for itself.
Perimeters gated (systematic DR-4 audit — one test each):
- /api/v1 core writes (session AND PAT) — RequireVerifiedEmail method
gate mounted after RequireAuth (server.go).
- Authenticated /auth/* mutations — token create/rotate/delete, PATCH
/me, 2FA setup/disable, OAuth link/unlink, and cli-session approve —
all fall through the method gate (no auth exemption); logout,
verify-email, resend-verification, delete-account allowlisted.
- Collab WS GET-upgrade — authorizeCollabAccess (a GET the method gate
can't catch; it persists Yjs edits).
- Remote MCP write path — dispatcher RequireVerifiedEmail hook fired in
buildAuthedRequest (the single chokepoint every synthesized write
passes through), wired in cmd/pad/main.go.
- OAuth-provider authorize + authorize/decide — emailUnverifiedBlocked
checks (mounted outside /api/v1; decide mints the auth code).
- POST /api/v1/import/url — SSRF/abuse surface, method gate.
Carve-outs: POST /api/v1/invitations/{code}/accept stays open for
unverified invitees (DR-1); legacy no-user workspace PATs are
intentionally ungated (currentUser==nil). Self-host (!cloudMode) is a
full no-op.
Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST
|
||
|
xarmian
|
b0eeef16ce |
feat(store): email_verification_tokens + SendEmailVerification + token reaper (TASK-1936) (#806)
Wave 2 of PLAN-1933 — verification-token infrastructure (pure infra; no endpoint consumes it until Wave 3). - Migration 071 (SQLite) / 049 (Postgres): email_verification_tokens table, cloning the password_resets shape (id/user_id FK/token_hash/expires_at/ used_at/created_at + token_hash + user_id indexes), per-dialect created_at default. - Store email_verification.go: 256-bit crypto/rand token, padver_ prefix, SHA-256-at-rest, non-destructive Lookup, atomic UPDATE...RETURNING Consume. Deltas from password_resets (DR-2): 24h TTL, keep invalidate-prior-on-mint (resend burns the old link), consume side-effect sets users.email_verified_at (RFC3339-with-Z, same format Wave 1's migration used) in one transaction — no password reset, no session mint. - Email SendEmailVerification: clones SendPasswordReset, "1 hour" -> "24 hours". - Token reaper (DR-5): lifecycle-safe background sweep (mirrors orphanGC/opLogGC — self-registers on Server.bg, context-cancellable via stop channel, started only from cmd/pad/main.go so unit tests don't leak goroutines) calling the four previously-unwired CleanExpired* methods (email verifications, password resets, sessions, CLI auth sessions) hourly. Adds CleanExpiredEmailVerifications. - Audit consts ActionEmailVerified + ActionEmailVerifiedByAdmin. Gates: make check + make test-pg green (store + migration on both dialects). Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
6a63fba188 |
feat(store): add users.email_verified_at column + model plumbing (TASK-1935) (#805)
Wave 1 of PLAN-1933 (email verification). Pure infra — nothing reads the column until Wave 3, so this is behaviourally a no-op and mergeable early. - Migration 070 (SQLite) / 048 (Postgres): add nullable email_verified_at TEXT, mirroring disabled_at. UNCONDITIONALLY backfill every existing row to verified (RFC3339 'Z'-suffixed) so no existing / OAuth / self-host account is write-locked on deploy (inverted vs password_set's conditional backfill). SQLite ALTER without IF NOT EXISTS; Postgres with it. - SAFE default = verified (DR-3): CreateUser / CreateOAuthUser write a verified timestamp unless UserCreate.Unverified is explicitly requested (only the future cloud self-serve branch will set that). A missed call site fails SAFE (verified), not write-locked. - models.User.EmailVerifiedAt + IsEmailVerified() (mirror IsDisabled). - Update userColumns + BOTH scan sites (scanUser AND the inline SearchUsers scan) so the admin user list keeps working. - Expose derived email_verified bool in sessionUserPayload for a later wave. Gates: make check + make test-pg both green (dual-dialect verified). Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
111e43d27f |
fix(server): invitation-preview endpoint + read-only email prefill on /join (BUG-1934) (#803)
On Pad Cloud the /join/[code] page never showed the invited email, so a
mistyped address hit a confusing 403 invitation_email_mismatch. Add a
non-consuming, public, always-200, rate-limited preview endpoint and wire
the join page to prefill the invited email read-only.
- GET /api/v1/invitations/{code}/preview returns {found,email,workspace_name,
has_account}. Reuses store.GetInvitationByCode (never accepts/consumes the
invite). Invalid/expired/missing codes and dangling-workspace codes all
return 200 {found:false} — no 404 status signal (enumeration safety). A
genuine DB fault still 500s (code-independent, leaks nothing).
- Public/pre-auth: added to isPublicAPIPath (matches only the trailing
/preview segment, so /accept stays auth-gated).
- Dedicated per-IP rate limiter (20/min, burst 20) wired into the RateLimit
switch so the endpoint can't be used to enumerate invite codes.
- TS client: api.members.previewInvitation + InvitationPreview type.
- /join page calls preview on mount, prefills + locks the invited email, and
defaults register-vs-login by has_account. Keeps the mode-switch affordance
and BUG-1930's register default when preview is unavailable.
- Tests: non-consumption, has_account, always-200 on unknown code, rate limit.
Composes with BUG-1930 (register default). Wave 0 of PLAN-1933 / IDEA-1927 §B5.
Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST
|
||
|
xarmian
|
ae62c097ca |
refactor(mcp): consolidate project next/standup/changelog onto REST endpoints (TASK-1916) (#802)
* refactor(mcp): consolidate project next/standup/changelog onto REST endpoints (TASK-1916) dispatchProjectNext/Standup/Changelog were a second server-side copy of the next/standup/changelog reshaping contract, written before TASK-1894 shipped dedicated REST endpoints for the same data. Replace the ~200 lines of duplicate reshaping with thin proxies that validate workspace (preserving the pad_set_workspace hint) and forward to GET /next|/standup|/changelog, relying on packageHTTPResponse's existing array-wrap (BUG-985) and the REST handlers' own days-default and per-status best-effort semantics rather than replicating them. dispatch_http_slice4.go is deleted; its unrelated dispatchLibraryActivate moves to dispatch_http_library.go now that the file's other three methods are gone. KEEP IN SYNC comments across handlers_project_intel.go/server.go/tests collapse from "three reproductions" to "CLI + REST, MCP proxies to REST." * fix(server): pass nav-lenient visibleIDs to changelog's parent enrichment (codex R1 P1, TASK-1916) handleGetProjectChangelog passed guestResourceFilter's narrowed collIDs into enrichItemsWithParent instead of the nav-lenient visibleCollectionIDs set handleListItems uses for the same enrichment call. For a guest whose granted item's parent lives in an item-grant-only collection (nav-visible but excluded from the narrowed full-access set), this silently dropped the parent link fields, causing itemMatchesParentFilter to exclude the item from ?parent= results even though the guest can otherwise see it. The root cause predates TASK-1916 (introduced alongside the REST endpoint in TASK-1894), but this consolidation imports it into MCP wire behavior via dispatchProjectChangelog's proxy, so it's in scope to fix here. projectIntelVisibility now returns the unnarrowed visibleCollectionIDs result (navVisibleIDs) alongside the existing (collIDs, itemIDs) pair; handleGetProjectChangelog uses navVisibleIDs for enrichItemsWithParent while keeping collIDs for the list query, mirroring handleListItems' pattern exactly. handleGetProjectStandup and handleGetProjectNext have no parallel enrichItemsWithParent call (verified by reading both, and buildDashboardResponse) so neither needed the same treatment. Added TestProjectChangelogEndpoint_GuestParentFilter_ItemGrantOnlyCollection, confirmed to fail against the pre-fix code and pass against the fix. |
||
|
xarmian
|
f3335adea2 |
fix(server): filter handleListUserGrants through caller visibility (BUG-1928) (#799)
handleListUserGrants returned a target user's raw collection/item grants (including collection_id/item_id) to any workspace owner unconditionally, letting a restricted owner (collection_access="specific") enumerate hidden-resource IDs — the disclosure half of the primitive BUG-1923's handlers closed the action half of. Filter the response through the caller's visibility when caller != target: collection grants against guestResourceFilter's strict full-access set (same set requireCollectionFullyVisible narrows to — item-grant-only collections don't qualify), item grants via a bulk item_id->collection_id lookup (GetItemCollectionRefs, state-agnostic so soft-deleted parents stay listed) plus the existing isItemVisibleToGuest set-membership check. Self-queries and unrestricted callers stay unfiltered, the latter via a cheap short-circuit. GetDeletedItemsWithCollection's query had no deleted_at filter despite its name; renamed the shared implementation to GetItemCollectionRefs and kept the old name as a wrapper for its existing delta-sync caller. |
||
|
xarmian
|
22ac55929a |
fix(server): gate ID-keyed grant/share-link handlers on target visibility (BUG-1923) (#798)
* Gate ID-keyed grant/share-link delete and view-history handlers on target visibility handleDeleteCollectionGrant, handleDeleteItemGrant, handleDeleteShareLink, and handleShareLinkViews operated on a grant/link ID directly with only a requireMinRole(owner) check — a restricted owner who knew an ID could revoke a grant or share link (or read its view-history) on a resource hidden from them by collection_access="specific". Unlike their slug-resolving create/list siblings (fixed in BUG-1920), these handlers never resolved the parent item/collection, so the visibility gate never ran. Resolve the grant/link's parent before acting: collection-scoped records go through the strict requireCollectionFullyVisible (no item-grant promotion, matching the minting/listing gates), item-scoped records through requireItemVisible. Item lookups use GetItemIncludeDeleted so a grant/link on a trashed-but-visible item stays revocable, matching handleListItemGrants' existing ResolveItemIncludeDeleted precedent. Fixes BUG-1923. * Fix soft-deleted-collection regression in BUG-1923's visibility gates Codex round 3 found that the BUG-1923 fix used GetCollection to resolve the parent collection for collection-scoped grant/share-link operations. GetCollection filters deleted_at IS NULL, but DeleteCollection soft-deletes and does NOT cascade-delete collection_grants or share_links — so the moment a collection was archived, its grants/share-links became permanently un-revocable and un-inspectable via the API (parent resolution 404'd before the visibility check even ran). Switch both call sites to the existing GetCollectionAnyState store method (no deleted_at filter; already used by the open-children guard for the same reason) instead of adding a new near-duplicate getter. Applied consistently to both handleDeleteShareLink and handleShareLinkViews via the shared requireShareLinkTargetVisible helper, so view-history stays readable for revocation decisions on an archived collection's link, not just the delete path. Confirmed via store trace: an unrestricted owner (visibleCollectionIDs == nil) is unaffected either way and is the main regression case, now fixed. A restricted owner's member_collection_access-derived visibility also survives the parent's soft-delete, since that lookup reads the raw table with no deleted_at join. |
||
|
xarmian
|
0c0a71f96b |
fix(server): bearer-gate canEditComment's admin bypass (BUG-1919) (#796)
canEditComment's unconditional u.Role == "admin" bypass let a bearer-authed (PAT/CLI/MCP) platform admin edit or delete any user's comment in a workspace where they're a member, contradicting the BUG-1616/1617 bearer-suppression intent (same family as BUG-1917/1918). Gate the bypass on !isBearerAuth(r), mirroring the idiom already used in handlers_collab.go's authorizeCollabAccess. Cookie-session admins keep the existing behavior, including editing empty-user_id legacy comments; bearer-authed authors can still edit their own comments. |
||
|
xarmian
|
d78efab167 |
fix(server): gate collection update/delete on visibility (BUG-1921) (#795)
handleUpdateCollection and handleDeleteCollection resolved a collection
by slug and gated only on requireMinRole("owner"), with no visibility
check afterward. A workspace-role owner independently restricted via
collection_access="specific" (member_collection_access has no role
exclusion, per BUG-1920) could rename, re-schema, or delete a
collection hidden from them.
Adds requireCollectionFullyVisible (added in BUG-1920 for the
share-link/grant minting twins) to both handlers, immediately after
the existing "collection not found" nil-check and after the pre-
existing requireMinRole 403 gate. This is the STRICT full-collection-
access variant, not handleGetCollection's nav-lenient
visibleCollectionIDs+isCollectionVisible check: an item-level grant on
a single item inside a hidden collection must not qualify as authority
to mutate the entire collection record. Restricted owners (session or
bearer) now get 404 on PATCH/DELETE of a hidden collection;
unrestricted owners are unaffected.
Downstream-consumer audit: every other collSlug-resolving handler
(views, artifact import, checkbox/child progress, item list/create,
bulk/single item move, grants, share-links) already gates on
visibility or edit permission. handleUpdateCollection/
handleDeleteCollection were the only ungated collection-record
mutation path.
Claude-Session: https://claude.ai/code/session_01CL1pBjNpPUX6SWkuAuYXHS
|
||
|
xarmian
|
d9b9dd9499 |
fix(server): gate share-link and grant minting on item/collection visibility (BUG-1920) (#794)
* fix(server): gate share-link and grant minting/listing on item/collection visibility (BUG-1920)
A workspace-role "owner" can be independently restricted via
collection_access="specific" (handleSetMemberCollectionAccess has no
role exclusion), but handleCreateItemShareLink, handleListItemShareLinks,
handleListItemGrants, and handleCreateItemGrant gated only on
requireMinRole("owner") with no visibility check afterward — letting a
restricted owner (any auth class) mint a public share-link token or a
grant for an item in a collection hidden from them, an exfiltration path
since share links are public-read. The collection-level twins
(handleCreateCollectionShareLink, handleListCollectionShareLinks,
handleListCollectionGrants, handleCreateCollectionGrant) had the same gap.
Adds requireItemVisible (existing, bearer-aware post BUG-1917/1918) to
the four item-resolving handlers, and a new requireCollectionVisible
helper (mirroring handleGetCollection's visibleCollectionIDs +
isCollectionVisible idiom) to the four collection-resolving handlers.
Restricted owners (session or bearer) now get 404 minting/listing
share-links or grants for hidden items/collections; unrestricted owners
and non-owners (403 via the pre-existing requireMinRole gate) are
unaffected.
Claude-Session: https://claude.ai/code/session_01CL1pBjNpPUX6SWkuAuYXHS
* fix(server): require full-collection access for share-link/grant minting (BUG-1920 R2)
Codex R2 caught a gap in the collection-level half of the previous
commit: VisibleCollectionIDs (used by requireCollectionVisible) folds in
collections that are visible ONLY via an item-level grant, "so the
collection appears in navigation" — intentional for handleGetCollection,
but it let a restricted owner holding nothing more than an item grant on
one item inside a hidden collection mint/list a share link or grant for
the ENTIRE collection.
Renames requireCollectionVisible to requireCollectionFullyVisible and,
mirroring reportVisibleCollections' fullCollIDs narrowing
(handlers_reports.go), restricts the acceptable set to full-collection-
access collections (collection grants + member_collection_access +
system collections) whenever the caller holds any item-level grants —
an item-grant-only collection no longer qualifies for collection-wide
minting/listing. handleGetCollection is untouched; its nav-lenient
check is intentional for metadata viewing. Item-level requireItemVisible
call sites are unchanged — an item grant legitimately entitles the
holder to act on that item.
Claude-Session: https://claude.ai/code/session_01CL1pBjNpPUX6SWkuAuYXHS
|
||
|
xarmian
|
da21598f10 |
fix(server): bearer-gate checkItemVisible's admin bypass (BUG-1918) (#793)
Bearer-authed platform admins who are restricted workspace members can no longer read, update, delete, or export a hidden collection's item by direct ref — checkItemVisible's unconditional admin bypass previously ignored isBearerAuth entirely, letting a bearer admin sidestep BUG-1917's list-level scoping for anyone who could guess a ref. Cookie session admins keep the existing unrestricted web-UI affordance. checkItemVisible gains an isBearer parameter (mirroring the existing authIsBearer idiom in resolverWorkspaceRole / guestResourceFilterCore) and gates its admin bypass on !isBearer. requireItemVisible's own signature is unchanged, so its ~20 call sites (comments, links, stars, versions, timeline, playbooks, backlinks, storage, artifact-export) inherit the fix for free; the three direct checkItemVisible callers (writeItemResolveError, handleBulkItems, resolverItemVisible) are updated explicitly. |
||
|
xarmian
|
a7f6fdf099 |
fix(server): bearer-gate visibleCollectionIDs to close BUG-1917 (#792)
Bearer-authed platform admins (PAT/CLI/OAuth) who are restricted members of a workspace were unrestricted on dashboard, bootstrap, items, and graph reads (plus item creation) — the last remaining gap in the BUG-1616/1617 pattern, where RequireWorkspaceAccess already suppresses the admin bypass for bearer auth everywhere else. visibleCollectionIDs now applies the same `!isBearerAuth(r)` gate as reportVisibleCollections, so a bearer admin who is only a scoped member is correctly restricted to their membership; a cookie-session admin keeps the existing unrestricted web UI affordance. Because this is the shared helper, every consumer (buildDashboardResponse -> /dashboard and /bootstrap, handleListItems, handleGetWorkspaceGraph, handleCreateItem's collection check, and ~25 other call sites) is fixed at once. This also completes TASK-1894's known asymmetry: standup's blockers/suggested_next sections (sourced from buildDashboardResponse) are now scoped for bearer admins just like its completed/in_progress sections already were. Folds the now-redundant bearerAwareVisibleCollectionIDs into the shared gate. Claude-Session: https://claude.ai/code/session_01CL1pBjNpPUX6SWkuAuYXHS |
||
|
xarmian
|
7c0b13767f |
feat(server): REST endpoints for project next/standup/changelog + WebMCP wiring (TASK-1894) (#791)
* feat(server): add REST endpoints for project next/standup/changelog
Adds GET /workspaces/{ws}/next, /standup, /changelog — session-authed
reads mirroring `pad project next|standup|changelog --format json`,
reusing buildDashboardResponse + store.ListItems so the browser
WebMCP surface stops returning "not available" for these catalog
actions (TASK-1894). Cross-references the MCP HTTP transport's
existing dispatchProjectNext/Standup/Changelog (dispatch_http_slice4.go)
with KEEP IN SYNC comments at both sites, since this is now a third
reproduction of the same reshaping contract pending a follow-up
consolidation.
* feat(web): wire next/standup/changelog into WebMCP dispatch + api client
Adds client.ts next()/standup()/changelog() methods and replaces the
three "not available in the browser" dispatch.ts stubs with real
handlers now that the backend endpoints exist (TASK-1894). Extracts
DashboardSuggestion as a shared type and adds StandupResponse /
ChangelogResponse types mirroring the Go response shapes.
* fix(server): make projectIntelVisibility bearer-aware (TASK-1894 codex R1)
standup/changelog's own item-list scoping used visibleCollectionIDs, which
has no bearer gate: a platform admin authenticated via a bearer token
(PAT/CLI/OAuth) who is only a restricted member of a workspace got the
unrestricted admin view instead of being scoped to their real membership.
Adds bearerAwareVisibleCollectionIDs, mirroring reportVisibleCollections'
existing BUG-1616/1617 gate, and switches projectIntelVisibility onto it
while preserving its item-level grant handling (which reportVisibleCollections
deliberately drops for aggregate reports).
buildDashboardResponse (and therefore /next, and standup's blockers/
suggested_next sections) is intentionally left ungated in this change —
gating it would break next's parity with dashboard.suggested_next and
diverge it from the CLI and MCP siblings. The resulting asymmetry is
documented inline pending a follow-up fix to buildDashboardResponse itself.
* docs(server): reference BUG-1917 in projectIntelVisibility comments
Replaces the textual placeholder ("the visibleCollectionIDs bearer-gate
bug filed from TASK-1894 review") with the actual bug number now that
it's been filed. Comment-only change, no behavior difference.
|
||
|
xarmian
|
0a1dbc8c73 |
perf(test): wire remaining store helpers onto storetest fixture (TASK-1915) (#789)
newPadServer (internal/mcp), testStoreOAuth (internal/oauth), and newMetricsTestServer (internal/server) were left on the slow per-call migration path after IDEA-1914/#788 wired testServer and store's white-box testStore onto storetest.NewSQLite. Switch all three, and add minimal TestMains to internal/mcp and internal/oauth to release storetest's process-wide template DB, matching internal/server's existing TestMain. |
||
|
xarmian
|
20544fdd44 |
perf(test): build the SQLite migration chain once per test binary (IDEA-1914) (#788)
* perf(test): build the SQLite migration chain once per test binary (IDEA-1914) internal/server's -race suite spent ~30 minutes replaying all 69 migrations + 3 backfills per test (~2.7s each, 622 store-backed tests, BUG-1913). Add internal/store/storetest, which runs the full migration chain once into a checkpointed, sidecar-free template DB (sync.Once) and hands every test a plain file copy opened via store.New. Wire it into internal/server's testServer/testServer_Stop_DrainsRateLimiterCleanup and internal/store's own testStore (duplicated inline there — an import cycle rules out sharing storetest with store's white-box tests). Postgres-mode tests are untouched. internal/server -race: 1819s -> 183s. * fix(test): plug template-dir leak and Cleanup race in storetest fixture Codex round 2 on IDEA-1914: buildTemplate/buildSQLiteTemplate left the MkdirTemp'd template dir on disk if store.New/checkpoint/journal_mode failed after mkdir succeeded — now removed via a disarm-on-success defer in both mirrored copies. Also guard Cleanup()/removeSQLiteTemplate against racing an in-flight build+copy with a sync.RWMutex (read-locked across build+copy, write-locked for removal) in both places. |
||
|
xarmian
|
e41ed8a236 |
fix(server): reserve parent/plan schema field keys (TASK-1912) (#786)
* fix(server): reserve parent/plan schema field keys (TASK-1912)
A collection schema field keyed exactly "parent" or "plan" makes the
parent-link extraction sites in handlers_items.go silently skip
fields-JSON extraction, disabling subtask linking with no error
anywhere. Reject newly-added occurrences of these keys on collection
create/update (grandfathering keys already present in a prior schema),
and add them to the web's reserved-key list so authors are steered
away before hitting the 400.
* fix(server): reject empty-string schema on collection PATCH (TASK-1912)
Codex round 2: handleUpdateCollection's validation guard was skipped
whenever input.Schema was a non-nil pointer to "", so a PATCH with
{"schema": ""} stored the empty string verbatim and every later
item-create against that collection 500'd instead of the mutation
being rejected up front. Drop the empty-string carve-out so "" flows
into json.Unmarshal, fails, and returns the existing 400 "Invalid
schema JSON". Omitting the schema field entirely (nil) is unaffected.
|
||
|
xarmian
|
584ac9a806 |
fix(web): treat CLI/MCP-created workspaces as agent-connected (BUG-1557) (#781)
`pad init` connects an agent (installs the skill, stores credentials) and creates a workspace, but the web UI still showed the "connect an agent" banner and onboarding launchpad. The only signal for "agent connected" was has_agent_activity — an item existing with source cli/mcp — and a fresh pad-init workspace has zero items, so the UI nagged to connect an agent the user already had. Give the server a truthful signal: a workspace created through an agent surface already has an agent wired up before it creates its first item. Add a `source` column to workspaces (web/cli/mcp), attributed authoritatively server-side from the request auth shape (actorFromRequest) — never from the request body, so a web client can't spoof "cli" to self-suppress the prompts. The dashboard ORs source in (cli,mcp) into has_agent_activity when the cheap item check comes up empty. - migrations 069 (sqlite) / 047 (postgres): workspaces.source NOT NULL DEFAULT '' (legacy rows stay "unknown", never treated as agent-created) - models.Workspace.Source + WorkspaceCreate.Source (json:"-", server-set) - thread source through the CreateWorkspace INSERT + all 7 workspace scan sites (workspaces.go, workspace_members.go) - handleCreateWorkspace derives source from actorFromRequest - OnboardingLaunchpad step 1 collapses to "Agent connected" when the agent is already wired up, shifting emphasis to "tell it to set up" Web modal and cloud-signup auto-create flows are unchanged and still correctly prompt to connect (source web / empty). Tests: store source round-trip across reads; dashboard reports agent-connected for a cli-created workspace with zero items; web-created stays not-connected until an agent item exists; a web body-spoofed source is ignored. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
9b9e2eb26b |
fix(admin): stop leaking 2FA secret via GET /admin/settings (BUG-1909) (#779)
platform_settings holds admin-managed UI keys alongside server secrets (2fa_challenge_secret — the 2FA challenge HMAC signing key) and plan_limits_* rows. handleGetPlatformSettings returned the whole table, so any admin client received the 2FA signing secret (and limit rows) in plaintext — a read-side secret exposure (not corruptible; the key isn't in the write whitelist, but it was fully exposed). Adopt deny-by-default: introduce adminManagedSettings, the canonical allowlist of the 8 admin-editable keys, and share it across read and write so they can't drift. GET now projects only those keys (masking maileroo_api_key); anything else in platform_settings — the 2FA secret, plan-limit rows, any future internal secret — is never exposed. PATCH gates writes on the same set. Tests: SecretsNotExposed (2fa_challenge_secret and plan_limits_* absent from GET, raw-body substring check, only allowlisted keys returned) and SecretNotWritable (PATCH can't overwrite the 2FA secret or write limit rows). A codebase-wide secret-exposure audit found no other confirmed leak surfaces. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
7e5917056a |
fix(admin): don't persist masked Maileroo API key on email settings save (BUG-1890) (#778)
* fix(admin): don't persist masked Maileroo API key on email settings save (BUG-1890) The admin settings "Save Email Settings" button PATCHed the whole platformSettings object. GET /admin/settings returns the Maileroo key masked (abcd...wxyz for >8 chars, **** otherwise), so saving without re-typing the key persisted the mask over the real key — silently breaking email until re-entered. Two layers: - Client (+page.svelte): track whether the API-key field was edited (apiKeyEdited flag) and scope the PATCH to the email fields this form owns (mirrors the TASK-1889 Integrations save). The key is included only when the admin actually edited it; an untouched save preserves the stored key, and clearing the field still sends "" to disable. - Server (handlers_admin.go): extract maskAPIKey() as the single source of truth for the mask format and skip persisting maileroo_api_key when the incoming non-empty value equals the mask of the currently-stored key. Best-effort backstop for non-web/old clients; the client fix is authoritative. Tests (handlers_admin_settings_test.go): maskAPIKey unit cases, the masked-key-not-persisted regression (both long and **** short masks), real-key-update-wins, and empty-key-clears. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST * fix(admin): clear Maileroo key when disabling email provider (BUG-1890) Codex review of the scoped email-save payload found a regression: when an admin selects Provider "None" without touching the key field, the scoped payload omitted maileroo_api_key, leaving the stored key. Because reconfigureEmail keys email enablement off the presence of the API key and ignores email_provider, "None" no longer disabled email. Send an explicit empty key whenever the provider isn't Maileroo, so disabling actually turns email off. The masked-key guard still applies when the provider is Maileroo and the key was left untouched. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST * fix(server): tear down live email sender when platform key is cleared (BUG-1890) Codex review: clearing the Maileroo key (e.g. disabling via provider "None") wrote the empty key to the DB, but reconfigureEmail's empty-key branch returned early without clearing the in-memory s.email sender — so the running process kept sending mail until restart, contradicting the UI's "disabled" state. Track whether email was wired from env vars (emailEnvConfigured, set in SetEmailSender). When platform settings carry no key, reconfigureEmail now tears down the live sender (s.email = nil, emailAPIKey = "") unless env config exists — env is the deployment baseline the admin UI doesn't disable. Tests pin both the teardown and the env-preserved paths. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |