mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-23 19:06:33 +00:00
134f55045d292b341ea7717b7163882832050da3
89 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
134f55045d |
feat(attachments): import workspace bundle with rehydrate + UUID remap (TASK-885) (#306)
* feat(attachments): import workspace bundle with attachment rehydrate + UUID remap (TASK-885) POST /workspaces/import now accepts a tar.gz bundle (Content-Type: application/gzip) and rebuilds the workspace + attachments + items in one round trip. JSON imports still work — content-type dispatch in handleImportWorkspace routes the request. Three-phase flow: 1. Walk the tar, capture pad-export.json + manifest.json + every attachment blob into memory. 2. Run the existing ImportWorkspace path to create the workspace + collections + items + comments + links + versions. New IDs are generated; item.slug is preserved (the existing remap path doesn't re-slugify). 3. For each manifest entry, rehydrate the blob through the storage backend (re-validate MIME + re-hash defensively, don't trust the manifest), insert a fresh attachments row. Build an oldID→newID map keyed on attachment uuid. 4. Walk every imported item's content + fields, replace "pad-attachment:OLD" with "pad-attachment:NEW" in one transactional pass. Refresh FTS afterward (direct UPDATE bypasses triggers). Phase 2 errors per-attachment are logged and skipped — the workspace keeps importing rather than rolling back. The import handler returns the new workspace and the operator can inspect logs for any attachment that didn't make it. CLI: - pad workspace export now defaults to --bundle (.tar.gz) since pad import handles bundles. --json reverts to legacy items-only. - pad import auto-detects format by file extension (.tar.gz / .tgz → application/gzip). Other extensions go through the legacy JSON path. - New Client.PostRawWithContentType for explicit-content-type POSTs. Tests: - TestImportBundle_RoundTrip: upload → embed in markdown → export source → import to FRESH server → verify attachment list has 1 row with new UUID → item content rewritten to new UUID and old UUID is gone → download new blob matches original bytes. - TestImportBundle_LegacyJSONStillWorks: JSON content-type still hits the legacy path. - TestImportBundle_RejectsBadGzip: garbage gzip body returns 400. Parent: PLAN-866. With TASK-884 + TASK-885 merged, the round-trip acceptance criterion (export → import → images intact) is met. * fix(attachments): stream import end-to-end per Codex (round 1) Two memory regressions Codex caught on PR #306: P1 (server). importBundle was buffering every blob into a map[string][]byte during a first pass, then iterating the manifest on a second pass. A 2 GiB bundle full of 25 MiB attachments would pin ~2 GiB of heap. Reworked to single-pass streaming: pad-export.json → import workspace + build slug→id map attachments/manifest.json → index entries by tar path attachments/<uuid>.<ext> → look up entry, rehydrate now The export bundler always writes pad-export.json + manifest.json BEFORE any blob (deterministic order from handlers_export_bundle.go), so this works without buffering. Bundles that violate the ordering — a third-party tool that writes blobs first — return 400 with a clear error. Memory footprint now bounded by the largest single blob (≤25 MiB) regardless of bundle size. Stale blobs without a manifest entry are skipped (their bytes io.Copy'd to io.Discard so the tar reader stays in sync). Unknown top-level entries (forward-compat for future bundle additions) are also consumed and ignored rather than left dangling. P2 (CLI). pad import used os.ReadFile, buffering the entire bundle client-side before posting. Switched to os.Open + a new Client.PostStreamWithContentType helper that streams the body directly into the request — together with the server-side fix, import is end-to-end streaming. Tests: - TestImportBundle_RejectsOutOfOrderTar: hand-crafted bundle with a blob before pad-export.json returns 400 with "ordering" in the message. - existing TestImportBundle_RoundTrip / LegacyJSONStillWorks / RejectsBadGzip continue to pass under the new streaming flow. * fix(cli): give streaming endpoints a 1h timeout per Codex (round 2) Codex P1 round 2: PostStreamWithContentType + RawStream were both using the shared 10s-timeout httpClient. The default works fine for normal API calls but kills a multi-GiB bundle import or export over anything slower than a local network — Client.Timeout fires mid-stream with "Client.Timeout exceeded". Added a dedicated streamClient on Client with a 1h timeout, used by both RawStream (export bundle download) and PostStreamWithContentType (import bundle upload). 1h is generous enough for ~100 MB/s uplinks shipping a 350 GiB bundle and still caps a hung connection eventually. The 10s default stays in place for every other call — short timeouts are the right SLA for normal API requests and protect the CLI from hanging on a wedged server. * fix(attachments): make import bundle cap configurable per Codex (round 3) Codex P1: the 2 GiB import cap was hard-coded with a comment promising operator override "later" — but no setter existed, so workspaces over 2 GiB stream out fine on export and fail on re-import. Added Server.SetImportBundleMaxBytes wired from the PAD_IMPORT_BUNDLE_MAX_BYTES env var in cmd/pad/main.go. Mirrors the existing PAD_ATTACHMENT_MAX_BYTES pattern. Default stays at 2 GiB so the typical workspace works without configuration; operators with larger exports can raise it without recompiling. The per-blob cap (importBlobMaxBytes = 25 MiB) is intentionally kept constant — it bounds in-flight memory regardless of total bundle size, and a 25 MiB-per-blob ceiling matches the upload handler's default, so a bundle can never smuggle larger blobs than the upload endpoint accepts. * fix(attachments): scale per-blob import cap with PAD_ATTACHMENT_MAX_BYTES per Codex (round 4) Codex P1 round 4: importBlobMaxBytes was hard-coded at 25 MiB but the upload handler's per-file cap is configurable via PAD_ATTACHMENT_MAX_BYTES. An operator who raised the upload cap to allow 50 MiB attachments could export a workspace successfully (WorkspaceAttachmentsForExport doesn't gate on size) but the re-import would reject every blob over 25 MiB. Replaced the const with effectiveBlobMaxBytes() which reads s.attachmentMaxBytes (or falls back to defaultAttachmentMaxBytes). The pad-export.json cap also scales with this value (4×) so a content-heavy workspace doesn't trip its own JSON ceiling on a server with raised attachment limits. Error message on a too-large blob now points the operator at PAD_ATTACHMENT_MAX_BYTES so they know which knob to turn rather than digging through code to find the cap. * fix(attachments): independent metadata cap for bundle import per Codex (round 5) Codex P2 round 5: tying pad-export.json + manifest.json caps to PAD_ATTACHMENT_MAX_BYTES regressed deployments that LOWER the attachment cap. A 1 MiB attachment cap would force metadata to fit in 4 MiB / 1 MiB respectively — but metadata size scales with workspace item count, not attachment blob sizes, so a tight upload limit shouldn't gate it. Added importMetadataMaxBytes = 100 MiB constant for both metadata files. effectiveBlobMaxBytes() still drives the per-blob cap which genuinely tracks attachment-upload policy. |
||
|
|
a0336e0248 |
feat(attachments): bundle attachments + manifest in workspace export (TASK-884) (#305)
* feat(attachments): bundle attachments + manifest in workspace export (TASK-884)
GET /workspaces/{ws}/export?format=tar streams a gzip'd tar bundle:
pad-export.json # the existing WorkspaceExport JSON
attachments/manifest.json # uuid → {filename, mime, size, hash, ...}
attachments/<uuid>.<ext> # original blobs only — no thumbnails
Default (no ?format) keeps returning JSON so existing automation
hitting the endpoint without a query param continues to work
unchanged. The CLI's pad workspace export now opts into the bundle
by default; pass --json for the legacy items-only output.
Implementation:
- store.WorkspaceAttachmentsForExport returns originals only
(parent_id IS NULL); thumbnails are re-derived on import via the
existing pipeline so shipping them would double the bundle size.
- handleExportWorkspaceBundle streams chunks straight into the
response writer rather than buffering — a workspace with multi-
GB of attachments would otherwise pin that much memory.
- AttachmentManifest is versioned (separate from WorkspaceExport
version) so the bundle layout can evolve independently.
- bundleAttachmentPath is exported (lowercase package fn) so the
import path in TASK-885 can resolve manifest entries to tar
entries without duplicating the filename logic.
- CLI gates against writing binary tar.gz to a TTY and appends the
conventional extension when -o is passed without one.
Tests:
- TestExportBundle_RoundTrip: two uploads → bundle contains
pad-export.json + manifest + 2 blobs whose bytes match the
uploads + manifest decodes cleanly + WorkspaceExport decodes.
- TestExportBundle_HidesThumbnails: synthetic thumbnail row, the
manifest excludes it.
- TestExportBundle_LegacyJSONStillWorks: no ?format param returns
application/json with a decodable WorkspaceExport (backward
compat regression guard).
Parent: PLAN-866. TASK-885 (import path + UUID remap) consumes the
manifest produced here.
* fix(attachments): stream export bundle + revert default to JSON per Codex (round 1)
Two findings from Codex on PR #305:
1. CLI buffered the entire response in memory via RawGet → io.ReadAll,
defeating the server-side streaming design and risking OOM on a
multi-GB bundle. Added Client.RawStream which copies the response
body straight into an io.Writer; export now opens the target file
and streams directly into it.
2. Default tar.gz output broke `pad export → pad import` round trip
because the import handler still only accepts JSON. Reverted the
CLI default to JSON; bundle is now opt-in via --bundle. The flag
docstring notes that TASK-885 will flip the default once import
handles bundles.
* fix(attachments): surface tar/gzip close errors and truncation per Codex (round 2)
Codex round 2 finding: deferred tw.Close() / gzw.Close() ignored
errors. If a backend returned fewer bytes than size_bytes claimed,
io.Copy returned nil, the tar writer's "missed N bytes" trip fired
at Close, and the handler still completed a 200 OK with a corrupt
bundle that gunzip would later refuse to decompress — silently from
the operator's perspective.
Two changes:
1. The deferred close now logs both tw.Close() and gzw.Close()
errors with structured context, so a corruption-on-finalize
trip shows up in the operator log.
2. streamAttachmentToTar checks the bytes-copied count against
a.SizeBytes after io.Copy and returns a per-attachment error
when they disagree. The error is logged with attachment_id +
storage_key so an operator can correlate the corruption with
the row to investigate.
Regression test: TestExportBundle_TruncatedBlobLogsError forces a
size_bytes/blob desync via direct UPDATE and asserts the resulting
bundle bytes don't decode cleanly. (HTTP status stays 200 because
headers are already on the wire by the time we detect the desync;
that's an inherent limitation of mid-stream errors, but the new
logs + close-error surfacing make the failure observable.)
* fix(attachments): X-Bundle-Status trailer for export-stream success per Codex (round 3)
Codex P1 round 3: even with the per-blob truncation log + tar/gzip
close-error logs, mid-stream failures looked successful to clients.
The CLI's RawStream finished without a transport error, the file
landed on disk, and "Exported workspace" printed regardless of
whether the bundle was actually complete.
Two complementary signals now mark a clean stream:
1. HTTP trailer X-Bundle-Status. The handler declares the trailer
in the initial Trailer header and sets it to "ok" only after
tw.Close() and gzw.Close() both return without error. CLI checks
the trailer after streaming and discards the file + returns
error if it's absent or non-"ok".
2. The handler skips the deferred clean close on the error path,
leaving the gzip footer unwritten. A client that ignores the
trailer (curl, third-party tooling) still sees a corrupt gzip
stream that gunzip refuses to decompress.
CLI: pad workspace export --bundle now removes any partial output
file on failure rather than leaving a corrupt one behind.
Client.RawStream signature changed to return (bytes, *http.Response,
error) so callers can inspect resp.Trailer; the only caller is the
export command.
Tests: TestExportBundle_TruncatedBlobAbortsStream now asserts both
signals (trailer absent + gzip/tar can't fully decode), and
TestExportBundle_SuccessTrailer pins the happy-path trailer.
|
||
|
|
335762c2bf |
feat(attachments): storage usage API + effective-limit computation (TASK-881) (#302)
* feat(attachments): storage usage API + effective-limit computation (TASK-881)
Adds GET /api/v1/workspaces/{ws}/storage/usage returning
{used_bytes, limit_bytes, plan, override_active}. Resolves the effective
limit through the existing three-tier chain (per-user override → platform
setting → hardcoded plan default) and surfaces the override flag for the
upcoming Settings → Storage and admin user-detail UIs.
Implementation:
- store.WorkspaceStorageInfo consolidates SUM(size_bytes) + owner-plan
resolution in one call; WorkspaceStorageLimit is now a thin wrapper so
the upload-time quota check and the API path stay consistent.
- Server.storageInfoCache is a 30s TTL memoizer to absorb repeated
Settings → Storage page loads. Invalidation hooks fire on upload,
thumbnail derivation, and transform — the ~30s eventual-consistency
window is bounded by TTL only when invalidation isn't reachable.
- Defensive copy on cache read so a caller mutating the returned struct
can't poison subsequent reads.
- New CLI command `pad workspace storage` prints "X used of Y (Z%)" with
IEC units (humanBytes helper) and surfaces the override flag.
- TS api.attachments.storageUsage() + WorkspaceStorageInfo type ready
for TASK-882's Settings → Storage page consumer.
Tests:
- Store-level: no-owner fallback, free-plan resolution chain, override
flip, pro-plan override-active visibility, soft-delete exclusion.
- Server-level: empty-workspace happy path, two uploads with cache
invalidation between, dedicated cache TTL/invalidate/copy-safety test.
Parent: PLAN-866.
* fix(attachments): gate storage usage on viewer+ per Codex review (round 1)
Codex correctly flagged that the storage/usage handler relied solely on
RequireWorkspaceAccess, which admits item-grant guests with
workspaceRole=="guest". Workspace-wide quota numbers (used_bytes, plan,
override status) shouldn't surface to guests — every other workspace-
level read handler uses requireMinRole("viewer") for exactly this case.
Adds the explicit gate + a regression test that calls the handler with
a guest-role context and asserts 403.
|
||
|
|
02be33902f |
feat(attachments): ImageProcessor interface + pure-Go impl + thumbnail pipeline (TASK-878) (#295)
* feat(attachments): ImageProcessor interface + pure-Go impl + thumbnail pipeline (TASK-878)
Adds the abstraction Phase 1 needs to derive thumbnail variants on
upload, with a pure-Go default implementation that keeps Pad's
single-binary distribution intact (no cgo). The libvips-tagged
build (Phase 2 / Pad Cloud Docker) will replace processor_purego.go
with a vips-backed implementation behind the same Processor
interface — see DOC-865.
internal/attachments/processor.go:
Processor interface — Decode(io.Reader)→(image.Image, format),
Resize(img, maxLong), Rotate(img, deg), Crop(img, rect),
Encode(img, format, w), Capabilities().
Capabilities struct (image_formats, can_transcode, max_pixels)
surfaces what the editor needs to gate per-format rotate/crop UI
on (TASK-879/880). ErrUnsupportedFormat + ErrImageTooLarge are
separate sentinels so callers can distinguish "format not
supported" from "image dimensions too big".
internal/attachments/processor_purego.go (//go:build !libvips):
Uses github.com/disintegration/imaging plus the stdlib decoders.
Supports PNG/JPEG/GIF/BMP/TIFF for all ops. WebP/AVIF/HEIC
reach Decode and bounce out via ErrUnsupportedFormat — uploads
still succeed (the MIME allowlist is the upload gate), but
thumbnails skip and the editor disables rotate/crop UI per
Capabilities.
Memory ceiling: Decode peeks via image.DecodeConfig (header only)
before allocating any pixel buffer and rejects images whose
width*height exceeds MaxPixelsDefault (8000² = 64MP). At 4 bytes
per pixel that caps the decode buffer at ~256 MiB and prevents an
attacker uploading a forged 100kx100k claim from OOMing the
server. The forged-CRC test exercises this gate.
internal/server/handlers_attachments_thumbnails.go:
deriveThumbnails(parentID) runs in goAsync after every image
upload. Generates thumb-sm (256px long edge) + thumb-md (1024px),
each as its own attachments row with parent_id pointing at the
original. Server.Stop() drains the goroutine before SQLite
closes, so tests can assert post-conditions deterministically.
Skip cases: parent deleted (race), source format not supported
(logged at debug), source already smaller than the variant's
bound, variant already exists (idempotent reruns). Variants
count toward workspace storage usage — DOC-865 is explicit about
this and TestThumbnails_CountsTowardWorkspaceUsage proves it.
Output format policy: PNG inputs stay PNG to preserve transparency;
everything else encodes as JPEG q=85.
internal/server/handlers_capabilities.go:
GET /api/v1/server/capabilities returns the Processor's static
capability profile under {image: {...}}. Public route — the
editor needs it before login (e.g. shared-item preview surfaces).
Reports an empty image-formats list when no processor is wired,
signalling the editor to disable rotate/crop UI rather than
500-ing the editor mount.
cmd/pad/main.go: wires SetImageProcessor(NewProcessor()) alongside
SetAttachments at startup; logs the supported formats so operators
know whether they're on the pure-Go or libvips build.
Tests:
- processor_test.go: 12 unit tests covering capability profile,
decode round-trip for PNG/JPEG/GIF, rejection of unsupported
formats and oversized images (forged-CRC PNG), resize aspect
preservation + pass-through for already-small inputs, rotate
multiples-of-90 + negative + 360-modulo handling, crop with
bounds clipping + empty-intersection rejection, encode round-
trip for PNG/JPEG, ThumbnailFormat/Mime/Ext policy.
- handlers_attachments_thumbnails_test.go: 5 integration tests
covering thumb-sm + thumb-md generation on PNG/JPEG uploads,
skip-when-source-already-small, ?variant=thumb-md serving via
the existing GET handler, workspace usage accounting.
- handlers_capabilities tests cover the happy path + the
no-processor degraded path.
Parent: PLAN-866. Closes the thumbnail-fallback gap that TASK-874 /
TASK-876 left open (thumb-md URLs were falling back to original
because no thumbnails existed). Unblocks TASK-879 (rotation tool)
and TASK-880 (crop tool) — both will reuse Processor.Rotate /
Processor.Crop with the same Capabilities-driven UI gating.
* fix(attachments): make /server/capabilities public per Codex review (round 1)
Codex flagged that GET /api/v1/server/capabilities was registered
inside the auth-gated API group but missing from isPublicAPIPath,
so once any user existed the editor's pre-login fetch would 401 —
contradicting the route's "public" register-time intent and breaking
the share-preview surface.
Fix: add the path to isPublicAPIPath. The handler is read-only,
returns a static profile, and has no per-user state, so making it
public has no security implication. Added
TestServerCapabilities_PublicAfterBootstrap as a regression guard:
it bootstraps an admin (so RequireAuth is active) and then fetches
the endpoint with no auth cookie, asserting 200.
* fix(attachments): make -tags libvips compile per Codex review (round 2)
Codex flagged that build tag !libvips on processor_purego.go meant
NewProcessor + the Thumbnail* helpers were absent under
\`go build -tags libvips\`, so cmd/pad/main.go and the thumbnail
handler — which call them unconditionally — broke that build.
Two minimal fixes preserving the documented Phase 2 split:
1. Move ThumbnailFormat / ThumbnailMime / ThumbnailExt out of the
tagged file and into processor.go (untagged). They're pure
format-name policy, not implementation specifics, so both
backends share the same definitions.
2. Add processor_libvips.go (//go:build libvips) with a stub
NewProcessor that panics at runtime with a clear
"Phase 2 hasn't shipped libvips yet" message. The libvips
build now compiles; anyone actually instantiating the
processor under that tag gets a loud failure rather than a
silent degradation. Phase 2 will replace the body with the
real govips-v2-backed implementation.
Verified: \`go build ./...\` and \`go build -tags libvips ./...\` both
clean. Existing tests still pass on the default tag.
* fix(attachments): make tests compile under -tags libvips per Codex review (round 3)
Codex flagged that running \`go test -tags libvips ./internal/attachments\`
or \`./internal/server\` panicked through the libvips NewProcessor
stub: processor_test.go and the thumbnail/capability server tests
all called NewProcessor() unconditionally, even though the libvips
build's stub is intentionally panicking until Phase 2 ships the
real implementation.
Three minimal fixes:
1. Tag processor_test.go !libvips. It tests the pure-Go
implementation specifically — there's no value in running it
under libvips, and the stub processor would explode the moment
NewProcessor() ran.
2. Tag handlers_attachments_thumbnails_test.go !libvips. Same
reasoning — these integration tests assert thumbnail
derivation against a working processor.
3. Split testServerWithAttachments's processor wiring into two
build-tagged helper files:
* testimageprocessor_purego_test.go (//go:build !libvips)
wires the real pure-Go processor.
* testimageprocessor_libvips_test.go (//go:build libvips)
is a no-op so the rest of the server test surface
(uploads, downloads, auth, etc.) compiles + runs cleanly
under -tags libvips.
Verification:
go build ./... — OK
go build -tags libvips ./... — OK
go test ./internal/attachments ./internal/server (default) — pass
go test -tags libvips ./internal/server -run "TestUpload|TestDownload" — pass
Phase 2 will introduce a real libvips test backend and drop the
!libvips tags on the thumbnail tests.
* fix(attachments): libvips binary boots cleanly per Codex review (round 4)
Codex flagged that the libvips build still crashed at \`pad serve\`
startup: cmd/pad/main.go calls attachments.NewProcessor()
unconditionally, and the libvips stub was panicking — so any
operator who built with -tags libvips today (Phase 2 isn't shipped
yet) lost the entire server, not just image processing.
Two minimal changes:
1. processor_libvips.go: stop panicking. Return nil + slog.Warn
instead. Every call site already nil-checks the processor (the
upload handler skips thumbnail derivation, the capabilities
endpoint reports a degraded empty formats list), so the
libvips-tagged binary now has the same runtime profile as a
self-host build that opted out of image processing entirely
— uploads succeed, originals display, only derived
transformations are unavailable. The slog.Warn keeps the
"this build doesn't have it yet" signal loud.
2. cmd/pad/main.go: skip srv.SetImageProcessor when NewProcessor
returns nil, and log a "not wired" message in that branch.
Distinguishes the wired vs. unwired states cleanly in the
boot log.
Phase 2 will replace processor_libvips.go's body with the real
govips-v2-backed implementation; main.go's wiring is already shape-
correct for that transition.
Verification:
go build ./... — OK
go build -tags libvips ./... — OK
go test ./... — pass (74s server tests included)
go test -tags libvips ./internal/server -run "TestUpload|TestDownload|TestServerCapabilities_Public" — pass
|
||
|
|
fc1c47f124 |
feat(attachments): CLI + TypeScript clients + types (TASK-873) (#290)
* feat(attachments): CLI + TypeScript clients + types (TASK-873)
Rounds out the API surface with Go and TS client methods + a
\`pad attachment\` Cobra subcommand for ops debugging.
internal/cli/client.go
AttachmentUploadResult struct mirrors POST /attachments JSON.
UploadAttachment streams a multipart file part via io.Pipe — never
buffers the upload in memory. itemRef is optional. Uses a fresh
http.Client with a 5-minute timeout per request so a 25 MiB upload
over a constrained link doesn't trip the package-shared 10s default.
DownloadAttachment streams the bytes into the caller's writer,
returning Content-Type + total bytes copied. Optional ?variant=
parameter for thumbnails (server falls back to original silently
per TASK-872).
cmd/pad/main.go
pad attachment upload <item-ref|-> <path> [--filename NAME]
pad attachment download <id> <out|-> [--variant thumb-sm|thumb-md]
Item arg accepts an issue ref (TASK-5) or slug; "-" means no parent.
Out arg "-" streams to stdout (with status messages on stderr) so
callers can pipe into image viewers etc. Resolves the item via
GetItem first so a typo'd ref fails fast with a useful error.
List + delete subcommands intentionally omitted — those endpoints
ship with TASK-881 (storage usage) and the future GC task. Adding
client methods that hit 404s would mislead callers; same logic kept
the upload response's "url" out of TASK-871 until TASK-872 wired GET.
web/src/lib/types/index.ts
Attachment interface mirroring the Go model (pointer types → optional).
AttachmentUploadResult interface for the upload response shape.
web/src/lib/api/client.ts
api.attachments.upload(workspaceSlug, file, itemId?) — multipart
POST via direct fetch (skips shared request() because that helper
hard-codes Content-Type: application/json). Carries CSRF, cookies,
and the same 401 → /login redirect.
api.attachments.downloadUrl(workspaceSlug, attachmentId, variant?)
is a pure URL builder so callers can wire <img src> directly without
going through fetch.
End-to-end smoke verified:
pad attachment upload TASK-869 /tmp/tiny.png # uploads PNG
pad attachment download <id> /tmp/dl.png # bytes are identical
cmp /tmp/tiny.png /tmp/dl.png # PASS
Verification
go build ./... — clean
go vet ./... — clean
go test ./... — all packages pass
cd web && npm run build — clean
make install — server restarts on the new binary
Parent: PLAN-866.
* fix(cli): atomic download — write to temp + rename so a failed download doesn't truncate the destination per Codex review (round 1)
P2: pad attachment download <bad-id> /existing/file used to wipe the
existing file on auth/network/404 errors because os.Create truncated
before the request was even attempted. The bytes were never written
because DownloadAttachment errored out, but the destination was
already 0 bytes — a footgun for anyone running the CLI in scripts.
Fix: for the file-path case, write to a sibling .tmp via os.CreateTemp
in the destination directory, fsync, close, then os.Rename only on
success. Same atomic-write pattern as FSStore.Put. The defer cleans
up the .tmp on any error path.
The stdout case (outPath == "-") is unchanged — bytes already
streamed to stdout can't be rolled back, so any partial write is
just visible to the caller as a short payload.
Verified end-to-end:
echo X > /tmp/existing.png
pad attachment download not-a-real-id /tmp/existing.png # errors
cat /tmp/existing.png # still "X" — file untouched
* docs(cli): clarify os.Rename atomic-replace behavior on Windows (Codex round 2 disagreement)
Round 2 flagged this as P2: "os.Rename does not replace an existing
destination on Windows." That is technically incorrect for modern Go.
Verified directly against the Go stdlib source:
src/internal/syscall/windows/syscall_windows.go:
func Rename(oldpath, newpath string) error {
...
return MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING)
}
MoveFileEx with MOVEFILE_REPLACE_EXISTING atomically replaces an
existing destination on Windows. This has been the behavior since
Go 1.5 (2015), so every version of Go this codebase supports already
gets the desired replace-on-rename semantics on every platform.
Added an inline code comment so future readers don't worry about the
same false alarm. No code-path change.
|
||
|
|
48b9e18d34 |
feat(attachments): upload API with MIME sniff + dedupe + quota tracking (TASK-871) (#288)
* feat(attachments): upload API with MIME sniff + dedupe + quota tracking (TASK-871)
Wires the upload endpoint that turns a multipart POST into an
attachments row plus a stored blob. Auth-gated (editor+), per-file
size cap, hash-streaming, MIME allowlist with extension blocklist,
fire-and-forget quota warning.
POST /api/v1/workspaces/{slug}/attachments
Multipart "file" field. Optional ?item_id=… or form item_id to
associate at upload time. Returns
{id, url, mime, size, width?, height?, filename, category, render_mode}.
Errors: 400 bad multipart, 400 empty file, 401 unauthorized, 403
insufficient role, 413 over per-file cap, 415 MIME or extension
rejection, 503 attachments not configured.
internal/attachments/mime.go
MIMEEntry + RenderMode + Category typed allowlist mirroring DOC-865.
Default-deny. SniffMIME wraps http.DetectContentType. ValidateUpload
cross-checks the sniff result against the filename extension and:
(a) rejects when the extension maps to a *blocked* MIME — covers
.svg (sniffs as text/xml; .svg ext makes the browser run embedded
<script>) and .exe family (sniffs vary; extension is unambiguous);
(b) rejects when the extension maps to an allowed MIME but the
sniff's category disagrees — the "exe pretending to be png" case.
Tests cover normalize/lookup/sniff plus happy path, exe-as-png,
extension mismatch, SVG, .exe-by-extension-alone, text/plain accept,
HTML force-download.
internal/store/attachments.go
CreateAttachment / GetAttachment / WorkspaceStorageUsage. Pointer
scan for nullables; SUM(size_bytes) excludes soft-deleted rows but
includes derived blobs (thumbnails are real bytes on disk).
internal/server/handlers_attachments.go
Body capped via http.MaxBytesReader BEFORE ParseMultipartForm spools
any of it. Streams "file" part into an os.CreateTemp file, sha256ing
in one io.MultiWriter pass — multi-GB POST never reaches RAM. Sniff
on first 512 bytes; image dimension probe via stdlib image.DecodeConfig
(PNG/JPEG/GIF). WebP/AVIF/HEIC accepted but width/height nil — matches
the "pure-Go gracefully degrades" decision in DOC-865. Calls
AttachmentStore.Put (which hash-verifies via the dedup fast path) and
inserts the row. Quota check (CheckLimit + WorkspaceStorageUsage) runs
in a goroutine — Phase 1 logs only; Phase 2 will enforce.
Anonymous uploads on a fresh install (RequireWorkspaceAccess grants
implicit owner without a current user) get uploaded_by="system".
internal/server/server.go
Server.attachments + attachmentMaxBytes fields and SetAttachments
setter. Route POST /workspaces/{slug}/attachments wired inside the
authenticated workspace block.
cmd/pad/main.go
Boot wiring: NewFSStore(<DataDir>/attachments) → Registry registered
under "fs" → SetAttachments. PAD_ATTACHMENT_MAX_BYTES env override
for the per-file cap.
Tests
internal/server/handlers_attachments_test.go covers:
happy path PNG (1x1, dimensions resolve to 1×1)
exe bytes with .png filename → 415
PNG bytes with .pdf filename → 415 (extension mismatch)
empty body → 400
missing file part → 400
over the size cap → 413
same content uploaded twice → two rows, same content_hash + storage_key,
WorkspaceStorageUsage = 2 × bytes (dedupe is at the blob layer,
not the row layer)
8 concurrent uploads of identical bytes → all 201, no corruption
no registry wired → 503
Verification
go build ./... — clean
go vet ./... — clean
go test ./... — all packages pass
make install — server restarts on the new binary
Parent: PLAN-866.
* fix(attachments): three Codex round-1 findings — drop premature url, accept Office docs, real quota probe
1. Upload response no longer returns "url". TASK-872 wires GET so any
URL we return today is a 404 — pulling it out keeps clients from
baking in the broken endpoint.
2. Office Open XML docs (.docx/.xlsx/.pptx) and OpenDocument formats
(.odt/.ods/.odp) are zipped XML — http.DetectContentType correctly
sniffs them as application/zip. Previously the validator's
extension-vs-sniff category check rejected them as
"mime_extension_mismatch" (archive vs document). Now: when the
sniffed type is exactly application/zip and the extension maps to
a document MIME, trust the extension and route to the document
entry. Plain .zip with the same bytes still routes to archive.
Test covers all six office/odf extensions plus the plain-zip case.
3. CheckLimit("storage_bytes") returned "unknown workspace feature"
because featureCount only knows row-counted features (items,
members, webhooks). The warning path silently dropped every probe.
Added Store.WorkspaceStorageLimit which does the same three-tier
resolution (user override → platform setting → hardcoded fallback)
but returns the limit only — usage is computed separately via the
existing WorkspaceStorageUsage. Self-hosted/pro plans return -1
(unlimited). Workspaces without an owner_id (fresh installs and
legacy rows) also return -1, so a fresh-install upload no longer
logs "owner not found". Switched maybeWarnStorageQuota to use
WorkspaceStorageLimit + WorkspaceStorageUsage directly. Now also
spawned via Server.goAsync so Stop() drains it (BUG-842 hygiene).
Tests
- TestValidateUpload_AcceptsOfficeOpenXMLAsZipBytes covers all six
extensions + plain .zip
- TestUpload_QuotaCheckResolves regression-tests finding 3: both
storage helpers return non-error after a real upload
- TestUpload_HappyPathPNG asserts the response no longer carries url
Verification
go build ./... — clean
go vet ./... — clean
go test ./... — all packages pass
* fix(attachments): trim trailing blank line at EOF in mime.go per Codex review (round 2)
Round 2 LOW: git diff --check flagged a "new blank line at EOF" on
internal/attachments/mime.go. Cosmetic but addressed because the
ship-tasks workflow requires zero findings (HIGH/MEDIUM/LOW alike) —
leaving LOWs unfixed compounds across PRs and prevents the loop from
ever converging clean on later work.
* fix(attachments): alias stdlib MIME-sniff quirks per Codex review (round 3)
http.DetectContentType returns names that don't match modern IANA
conventions for two formats on the allowlist:
audio/wave → audio/wav (.wav uploads)
application/x-gzip → application/gzip (.gz uploads)
Without aliasing, valid uploads of either format hit "mime_not_allowed"
because the allowlist uses canonical names. Added a sniffAliases map
applied inside SniffMIME so allowlist lookups always see the canonical
form. Allowlist stays single-sourced; the fix is one map entry per
quirk we discover.
Tests:
- TestSniffMIME_AliasesStdlibQuirks pins both aliases at the sniff layer
- TestValidateUpload_AcceptsWAV / TestValidateUpload_AcceptsGzip verify
the end-to-end accept path with real WAV (RIFF/WAVE) and gzip headers
|
||
|
|
a03c96f9b0 |
feat(cli): pad init --url X --workspace <slug> as web-first cold-start (TASK-860) (#282)
Make `pad init --url <server> --workspace <slug>` a reliable non-interactive cold-start so the web UI can hand users a single copy-paste command to connect a workspace they created on the web to their local project. Keystone CLI work for the web-first onboarding on-ramp under PLAN-859 (driven by IDEA-750). Changes: - `ensureWorkspace` gains a `wsSlug` parameter. When set, it ONLY attaches by slug — looks up the workspace via GetWorkspace, links the CWD if found, and surfaces a clear "not found on <server>" error otherwise. Critically, it never silently falls through to creating a new workspace named after the slug. - Refuses to clobber a CWD that's already linked to a different workspace; idempotent re-run when the existing link matches. - `pad init --url X` on a fresh machine (no config.toml on disk) now persists the config so subsequent commands don't need --url. - When both a positional name and --workspace are supplied, the slug wins and we print a Note: line so the override is visible. - Same wiring applied to `pad workspace init` for consistency. Tests: 5 new unit tests in cmd/pad/init_test.go cover slug-attach, not-found error, clobber refusal, idempotent re-run, and that the legacy name-driven path still works. Smoke-tested end-to-end against the local server: happy path links, missing slug errors cleanly with no `.pad.toml` written, clobber blocked, idempotent re-run silent. |
||
|
|
0fd5d0cdfb |
fix: green up Go (PostgreSQL) CI (BUG-842) (#275)
* fix(store): swap plainto_tsquery → websearch_to_tsquery for PG FTS (BUG-842)
`TestListItems_FTS_HyphenatedSearchTerm/task-five` has been failing on
every Go (PostgreSQL) CI run because `plainto_tsquery('english',
'task-five')` doesn't match the asciihword lexeme(s) the english parser
produces for an indexed `task-five-distinctive`. The result is that
every PG full-text search for hyphenated terms returns zero rows.
`websearch_to_tsquery` (Postgres 11+) is purpose-built for arbitrary
user input and tokenizes hyphenated terms the same way `to_tsvector`
does for the indexed document, so the query intersects the index
correctly. Swapped in three spots in the postgres dialect — FTSMatch,
FTSSnippet, FTSRank — and updated the caller-side comments that
referenced plainto_tsquery. SQLite path is unchanged: it goes through
items_fts MATCH with sanitizeFTSQuery, never through these methods.
* fix(server): drain background goroutines on Stop() (BUG-842)
`TestAdminBillingStats_SidecarSidecarError_DegradesToLocalOnly` (and
other server tests) have been flaking on the Go (PostgreSQL) CI runner
with `TempDir RemoveAll cleanup: directory not empty`. Root cause:
several request handlers spawned bare `go func() { ... }()` goroutines
that touched the SQLite WAL DB after the test function returned.
testServer's t.Cleanup closed the store but had no way to drain those
goroutines first, so a fire-and-forget WAL write could re-create the
`-wal`/`-shm` files between Close() and t.TempDir's RemoveAll.
Add a Server.bg sync.WaitGroup, a Server.goAsync helper that wraps a
WaitGroup-tracked goroutine, and a Server.Stop() that blocks until
every goAsync closure has finished. Convert the four known
fire-and-forget sites to goAsync:
- middleware_auth.go (TouchUserActivity)
- handlers_auth.go (password reset email)
- handlers_cloud.go (stripe_processed_events pruning)
- handlers_members.go (workspace invitation email)
Wire `srv.Stop()` into both testServer (server_test.go) and
newMetricsTestServer (metrics_auth_test.go) so cleanup order is
Stop → Close → TempDir RemoveAll. Add
TestServer_Stop_DrainsBackgroundGoroutines to pin the contract: a
goAsync goroutine must block Stop until it returns.
* fix(store): correct PG FTS hyphenation via OR-combined plainto_tsquery (BUG-842)
The previous attempt swapped plainto_tsquery → websearch_to_tsquery,
which was wrong: websearch_to_tsquery treats `-` as a NEGATION operator
(Google-style), so `task-five` becomes `task & !five` and the search
returns 0 rows for the same reason as before. This commit reverts the
swap and applies the actual fix.
PG's english parser indexes `task-five-distinctive` as
`{task-five-distinct, task, five, distinct}` — the asciihword AND its
parts. plainto_tsquery applied to the partial query `task-five`
produces `task-fiv & task & five`: the stemmed asciihword for the
PARTIAL query (`task-fiv`) is NOT in the vector, so the AND fails.
Replacing the hyphen with a space makes plainto emit `task & five`,
which DOES match — but doing that unconditionally breaks `BUG-842`-
style queries: PG indexes the `-842` suffix as a negative-number
lexeme, so `plainto_tsquery('BUG-842')` matches via `-842`, while
`plainto_tsquery('BUG 842')` searches for `842` and misses.
The fix ORs the two query variants together so the search vector is
matched against either the raw user query OR its hyphen-as-space form.
Both `task-five` (against `task-five-distinctive`) and `BUG-842`
(against `BUG-842 fix the cleanup race`) hit. Verified locally against
postgres:17-alpine via PAD_TEST_POSTGRES_URL — both 10x stress and
race-detector runs are green.
Surfaces:
- dialect.go: FTSMatch / FTSSnippet / FTSRank now consume TWO
placeholders each in the PG dialect.
- items.go: listItemsFTS PG branch + SearchItems PG branch update
args to pass (raw, sanitized) for every PG `?` placeholder.
- search.go: SearchItems main / count / facets PG branches updated
likewise. New sanitizePGFTSQuery helper alongside sanitizeFTSQuery.
- documents.go: ListDocuments PG branch updated.
Tests:
- TestListItems_FTS_HyphenatedSearchTerm extended with a `BUG-842`
case to pin the OR-combined logic — naive hyphen-stripping would
silently regress this.
- New TestSanitizePGFTSQuery unit test.
* chore: gofmt 11 files with import-order issues (BUG-842 PR cleanup)
The Go (SQLite) CI job has been failing on `main` (and every PR built
against it) because golangci-lint flags 11 files whose third-party
imports are intermixed with internal imports — the import-grouping
rule that gofmt enforces. None of these were introduced by the
BUG-842 PR; they're pre-existing on main. The PR can't go green
without this cleanup, though, so it's bundled here.
Pure mechanical change — `gofmt -w <files>` only re-orders import
groups; no logic changes. Files touched:
cmd/pad/configure.go
cmd/pad/main.go
internal/cli/format.go
internal/server/handlers_admin_invitations.go
internal/server/handlers_admin_users.go
internal/server/handlers_grants.go
internal/server/handlers_share_links.go
internal/server/handlers_stars.go
internal/server/middleware_auth.go
internal/store/store.go
internal/store/store_test.go
After this commit `gofmt -l ./cmd ./internal` returns clean.
|
||
|
|
7cda0d7896 |
feat: rebrand to Perpetual Software + new tagline (IDEA-832) (#273)
Migrates from xarmian/pad to PerpetualSoftware/pad across the entire
repo and updates the product subtitle to "Collaborate with your AI
agents".
Go module rename
- go.mod: github.com/xarmian/pad → github.com/PerpetualSoftware/pad
- All Go imports updated across cmd/pad, internal/{cli,server,store,
models,collections,items,events,metrics,webhooks} (~130 files)
- Test fixtures with the literal repo slug ("xarmian/pad" in JSON
shapes, SSH/HTTPS git URL strings, workspace_context fixtures)
also updated, including the secondary repo entry
(xarmian/pad-web → PerpetualSoftware/pad-web — pad-web was also
moved to the org per branch context)
Docs / config
- README badges, install instructions, brew tap, Docker image, source
build path, sponsor link (sponsor link kept as personal @xarmian)
- Subtitle: "Project management for developers and AI agents." →
"Collaborate with your AI agents." (README, manifests, web layout
meta, .goreleaser homebrew description)
- CONTRIBUTING.md, SECURITY.md, skills/INSTALL.md
- .goreleaser.yaml: homebrew_casks owner, GHCR image, release github
owner, cosign cert-identity regex, comments
- .github/workflows/release.yml: tap/release comments
- deploy/k8s/deployment.yaml: container image
- docs/deployment.md: clone URL
- web/static/{site.webmanifest,manifest.json}: description
- web/src/routes/+layout.svelte: meta description + og:description
Brew tap path is PerpetualSoftware/tap/pad (CamelCase, matches
GitHub user case). GHCR image is ghcr.io/perpetualsoftware/pad
(lowercased per GHCR's URL normalization). CODEOWNERS @xarmian and
FUNDING.yml github: xarmian intentionally retained — those are the
personal maintainer / sponsor account, separate from the org repo.
Verification: go build ./..., go test ./... (all pkgs pass), web
build, and make install all clean (TASK-844, TASK-845).
|
||
|
|
afe721d202 |
feat(cli): add Cloud mode to pad init, drop Docker option (TASK-837, TASK-838) (#272)
Merging despite Go (PostgreSQL) red — those failures (TestListItems_FTS_HyphenatedSearchTerm/task-five + TestAdminBillingStats_SidecarSidecarError_DegradesToLocalOnly TempDir cleanup race) are pre-existing on main and tracked in BUG-842. Codex reviewed in 3 rounds (round 1 clean → round 2 found a real semantic bug → fix → round 3 clean). Tests, vet, and lint all green; remaining check failures are documented pre-existing. |
||
|
|
afd3b3c5ee |
feat(cli): explicit cancel + clean SIGINT for pad init prompts (TASK-835) (#270)
* feat(cli): explicit cancel + clean SIGINT for pad init prompts (TASK-835) The interactive prompts in 'pad init' / 'pad workspace init' previously relied on Go's default Ctrl+C behavior (terminate with no message) and offered no in-prompt way to back out. A user who realized mid-init that they were in the wrong directory had no clean exit and risked partial state. This change: - Adds cmd/pad/cancel.go with errCancelled (sentinel), cancelInit() (the canonical "Cancelled." + os.Exit(130) path), and an installable SIGINT/SIGTERM handler. - Template picker accepts c/q/cancel/quit (case-insensitive) and returns errCancelled. Prompt text now mentions the cancel option. - Mode picker (pad configure) gains the same cancel keywords + prompt hint. - Both pad init and pad workspace init RunEs install the signal handler and convert any propagated errCancelled into the same cancelInit() exit, using a named-return + LIFO defer so the existing body is unchanged. - SilenceErrors + SilenceUsage are set on both init commands so cobra doesn't render an "Error: cancelled by user" line on top of our friendly message. State on cancel: the template picker is invoked AFTER step 1 (configure) but BEFORE the workspace is created on the server and BEFORE .pad.toml is written, so an abort at that prompt leaves no half-created workspace, no orphan .pad.toml, and no stale credentials. Tests cover all cancel keyword variants (both via the picker and via errors.Is on wrapped errors), and verify the prompt surface mentions the cancel option. Parent: PLAN-833. Source: IDEA-831 issue #4. * fix(cli): wire cancellation into all init paths per Codex review Round 1 findings: - HIGH: getConfiguredConfig() called os.Exit(1) on errCancelled, bypassing the canonical "Cancelled." + 130 exit. It now recognizes the sentinel and routes through cancelInit() before falling through to its generic Error path. - HIGH: SilenceErrors+SilenceUsage on the init commands silenced every error, hiding real failures (e.g. server connection problems). Removed both — cancelInit() never returns to cobra, so the cancellation case doesn't need silencing, and real errors print normally again. - MEDIUM: doBrowserLogin returned `fmt.Errorf("login cancelled")` on context cancel, which is not errCancelled. If its inner signal listener won the race against the outer init handler on Ctrl+C, the propagated error didn't match isCancellation and the command exited 1 with a generic message. doBrowserLogin now returns errCancelled directly so whichever goroutine wins the race, the exit converges on 130. - MEDIUM: cancel.go cleanup race — if a signal arrived between init completion and the goroutine returning, both sigCh and done could be ready and select could pick sigCh, turning a successful run into a spurious 130 exit. Added a re-check on done inside the sigCh branch so late signals are suppressed once cleanup has run. Also reordered cleanup to call signal.Stop before close(done) so no new signals enter the buffer during shutdown. - MEDIUM: promptForValue (the URL prompt for remote/docker mode) still treated 'c' as URL input and failed validation. It now recognizes c/q/cancel/quit and returns errCancelled, matching the picker and mode-prompt behavior. LOW finding (account-setup prompts) intentionally not addressed: Ctrl+C already covers them via the outer handler, and explicit keyword recognition on the password prompt would risk collision with real passwords. doInteractiveLogin is not on an init path. Parent: PLAN-833. * fix(cli): cancel sentinel handling for pad auth configure / pad auth login Codex round-2 findings: - MEDIUM: pad auth configure RunE returned errCancelled directly to cobra. Now wraps the body with the same isCancellation -> cancelInit() deferred check used in pad init, so 'c' at the mode/URL prompt exits with the canonical "Cancelled." + 130. - LOW: pad auth login RunE called doBrowserLogin (which now returns errCancelled on signal cancellation). The sentinel was leaking to cobra. Added the same deferred check so SIGINT during browser login exits 130 with a friendly message regardless of which goroutine wins the cancellation race. Neither command installs the outer SIGINT handler — pad auth login relies on doBrowserLogin's existing inner listener (avoiding the double-listener race) and pad auth configure's prompts are short enough that Go's default Ctrl+C handling for those is acceptable. The new deferred checks just plug the sentinel-leak holes. Parent: PLAN-833. |
||
|
|
189b22825e |
fix(cli): use cfg.BaseURL()/BrowserURL() in pad init success message + pad open (TASK-834) (#269)
* fix(cli): use cfg.BaseURL() in pad init success message (TASK-834) The "Or open the web UI at http://localhost:7777" line in printOnboardingHints was hardcoded, which is wrong for any non-local connection mode (Remote, Docker, eventual Cloud). The CLI already knows the configured base URL — it just used it to talk to the server. Same hardcoded URL existed in the workspace-onboard skip path ("You can activate conventions from the library: ..."). Both call sites now use cfg.BaseURL(), which yields the correct URL for every mode: - Local: http://127.0.0.1:7777 (default host:port) - Remote/Docker/Cloud: the configured URL (e.g. https://app.getpad.dev) printOnboardingHints now takes a *config.Config; both call sites already had cfg in scope. Parent: PLAN-833 (pad init UX gaps + Pad Cloud onboarding fixes). Source: IDEA-831 issue #5. * fix(config): add BrowserURL() that normalizes 0.0.0.0 to 127.0.0.1 Per Codex review (round 1): when local mode runs with --host 0.0.0.0 (bind-all), cfg.BaseURL() returned "http://0.0.0.0:7777" — a bind address that browsers don't reliably accept. BrowserURL() behaves like BaseURL() except that when constructing from host:port, an unspecified bind-all host (empty, "0.0.0.0", "::", "[::]") is rewritten to "127.0.0.1". Explicit URL configurations (Remote/Docker/Cloud) are returned unchanged. The two onboarding-hint call sites updated in the previous commit now use BrowserURL() so the success message and skip-path show a clickable URL in every supported configuration. Tests cover loopback, named hosts, empty/0.0.0.0/::/[::] normalization, and explicit-URL precedence. Parent: PLAN-833. * fix(cli): use BrowserURL() in pad open for bind-all safety Per Codex review (round 2): the 'pad open' command prints and opens cfg.BaseURL(), which produces 'http://0.0.0.0:7777' when the local server is bound bind-all. Same class of bug as the onboarding hint fix in this PR — switch to cfg.BrowserURL() so the URL is a usable browser destination. A second related issue Codex flagged — the server-issued CLI auth URL in doBrowserLogin (which goes through internal/server/handlers_cli_auth.go using r.Host) — is a different surface with multiple possible fix strategies and overlaps with the post-v0.1.0 OAuth-architecture work. Deferred to TASK-839 with a written-up runbook so it isn't lost. Parent: PLAN-833. |
||
|
|
e5e2bd7b86 |
chore: flip CI only-new-issues=false + scope lint policy (TASK-771) (#253)
* chore: gate CI on full lint, scoped to checks we enforce (TASK-771) Flip golangci-lint-action's only-new-issues from true to false so CI fails on ANY linter finding, not just findings on PR-changed lines. This catches lint regressions on the next push instead of letting them drift into main. The gate flip is paired with a deliberate scope-down of .golangci.yml: 1. errcheck is disabled. The codebase has 325 pre-existing unchecked- error sites where the error is intentionally discarded (best-effort logging writes, defensive parses with zero-valued fallbacks, etc.). Auditing every site is its own project — bigger than IDEA-732 by an order of magnitude. Tracked as a follow-up if/when we want the safety net back. 2. staticcheck is restricted to the SA* check family (real-bug detectors). The ST*/QF*/S* families are stylistic/quick-fix suggestions we don't gate CI on yet — they would have re-flooded the lint output with capitalized error strings, De Morgan's law simplification suggestions, etc., that aren't bug-finding signals. Re-enable selectively if the team wants them. After scoping, the live linters are: govet, ineffassign, staticcheck (SA*), unused, gofmt — exactly the set that IDEA-732 cleaned up. Other changes in this PR: - Drop pull-requests:read permission. It was only required by the golangci-lint-action when only-new-issues=true (the action used it to fetch PR diff metadata). Not needed any more. - Update the Run-golangci-lint comment block to explain the new policy and reference the IDEA-732 cleanup PRs (#247/#249/#251/#252). - Replace the SA4017 //lint:ignore directive in cmd/pad/main.go:4631 with an inline //nolint:staticcheck — the multi-line //lint:ignore block was too far from the if statement for staticcheck's proximity rule, so the directive wasn't taking effect. - Apply gofmt -w on three files where post-deletion blank-line artifacts had drifted (cmd/pad/main.go imports, two trailing newline fix-ups in handlers_items.go and middleware_ratelimit.go). Verified: - `golangci-lint run ./...` reports 0 issues. - `go build ./...` clean. - `go vet ./...` clean. - `go test ./...` all pass. Parent: PLAN-644. * chore: address Codex round 1 on PR #253 (TASK-771) Two LOWs from Codex on the gate-flip PR: 1. //nolint:staticcheck was broader than necessary (suppressed any future staticcheck diagnostic on the line) and didn't self-report when the underlying false positive gets fixed upstream. Codex suggested swapping back to a tightly-placed //lint:ignore SA4017. I tried that, but golangci-lint v2's staticcheck integration does not honour //lint:ignore the way direct staticcheck does — the directive was silently no-op'd via golangci-lint while the same directive worked when staticcheck was invoked directly. So instead of fighting the linter wrapper, sidestep the false positive entirely: rewrite the keepalive check from `strings.HasPrefix(line, ":")` to `len(line) > 0 && line[0] == ':'`. Same observable behaviour for a single-byte ASCII prefix, no suppression directive needed at all, no exposure when staticcheck eventually fixes the false positive. 2. The new lint-step comment in ci.yml said main is "clean of staticcheck SA*/U1000" — but U1000 is reported by the standalone `unused` linter in .golangci.yml, not by staticcheck.checks. Tighten the comment to attribute each enforced check correctly. Verified: - `golangci-lint run ./...` reports 0 issues - `go test ./cmd/pad/...` passes (the SSE watch loop is exercised by reconcile_test.go and the broader integration tests). |
||
|
|
bf5ab5b366 |
chore: clear staticcheck SA + U1000 findings on main (TASK-764) (#249)
* chore: clear cosmetic staticcheck findings (TASK-764)
Apply zero-behavior-change fixes for 8 staticcheck findings on main:
- SA4023 cmd/pad/main.go:431 — drop always-true `if eventBus != nil`
guard. eventBus is wrapped in metrics.NewInstrumentedBus a few lines
above, which returns a concrete *InstrumentedBus that is never nil.
- SA1019 cmd/pad/main.go:3926 — replace deprecated strings.Title with
golang.org/x/text/cases.Title(language.English).String. golang.org/x/text
was already an indirect dep; now promoted to direct.
- SA4031 internal/server/handlers_changes.go:130 — delete dead
`if updatedItems == nil { ... }` block. make([]T, n) always returns
non-nil; the JSON marshalling already produced [] not null.
- SA9003 cmd/pad/init.go:351 — delete empty if branch and fold its
intent into the surrounding comment.
- SA9003 internal/server/handlers_dashboard.go:125 — replace empty
`if err == nil { ... }` branch with `_ = json.Unmarshal(...)` to
match the sibling settings parse and document the best-effort intent.
- SA4006 internal/cli/format.go:153 — drop the dead initial
`titlePart := item.Title` (overwritten in both branches below);
declare titlePart with `var` instead.
- SA4006 internal/store/workspaces.go:70 — drop the dead first call
to s.uniqueSlug; only the workspace-specific uniqueWorkspaceSlug
is meaningful (workspace slugs are globally unique, not workspace-
scoped like collection/item slugs).
- SA4000 internal/store/store_test.go:99 — remove always-true outer
`if idx := len(connStr) - len(connStr); idx >= 0` and unindent the
inner '?' query-string split.
go.mod side effects from `go mod tidy` under Go 1.26: golang.org/x/text
moves to direct (used directly now); pquerna/otp, prometheus/client_*
and trustelem/zxcvbn move from indirect to direct (they were already
used directly — Go 1.26's tidy correctly classifies them).
Verified:
- `go build ./...` clean
- `go vet ./...` clean
- `go test ./...` all pass (including the replaceDBName test path)
- `staticcheck -checks "SA1019,SA4000,SA4006,SA4023,SA4031,SA9003"` clean
except for handlers_dashboard.go:221 (SA4006, dashboard visibility-
filter dead block — handled in TASK-765)
Parent: PLAN-644.
* fix: clear SA5011 nil-deref in buildReconcileFindings (TASK-764)
extractItemStatus(item.Fields) on the first line of the function would
have panicked on a nil item before the `if item != nil && item.CodeContext
== nil` guard could fire. Staticcheck SA5011 flagged the inconsistency.
Drop the (item != nil) half of the guard — the function now documents
its non-nil contract in the doc comment. All callers (reconcile.go:204
plus three sites in cmd/pad/reconcile_test.go) already pass non-nil,
so this is documentation, not behaviour change.
Verified:
- `go build ./...` clean
- `go test ./cmd/pad/...` passes (the existing reconcile tests cover the
contract)
- `staticcheck -checks SA5011 ./...` clean
Parent: PLAN-644.
* chore: silence SA4017 false positive in watchCmd SSE loop (TASK-764)
cmd/pad/main.go SSE keepalive branch:
if strings.HasPrefix(line, ":") {
continue
}
Staticcheck SA4017 reports "HasPrefix doesn't have side effects and
its return value is ignored" — but the return value IS used as the
if condition. Two sibling strings.HasPrefix calls earlier in the same
for-loop body (matching "event: " and "data: " prefixes) are not
flagged, which strongly suggests an SSA-analysis quirk specific to
this branch rather than a real defect.
Suppress the finding with a //lint:ignore directive that explains
the false positive in-place. Rewriting to a different form (extract
to a bool var, comma-OK on a synthetic value, etc.) would be uglier
than the suppression comment.
Verified:
- `staticcheck -checks SA4017 ./...` clean
- `go build ./...` clean
Parent: PLAN-644.
* chore: delete dead code flagged by U1000 (TASK-764)
Pre-launch (no external contributors yet) — no consumer fork can be
relying on these unreferenced symbols, so we delete them rather than
carry the maintenance burden into v1.
## Helpers (14 functions, 1 type)
cmd/pad/main.go
- progressBar — never called
internal/cli/format.go
- stripHTMLTags — never called
internal/server/handlers_dashboard_test.go
- updateItem (test helper) — never called from any test
internal/server/handlers_items.go
- publishItemEvent — wrapper over publishItemEventWithName; all 5 call
sites use the *WithName variant directly.
- resolveRelationFields — never called.
- resolveRelationFieldFiltersForWorkspace, resolveRelationFieldFilters,
relationFilterKeys, resolveRelationFilterValue — closed loop of dead
helpers (each one only called by another dead one in the family).
- extractStatus — never called (cmd/pad/reconcile.go has its own copy).
internal/server/handlers_versions.go
- handleGetDiff (HTTP handler) — never wired into setupRouter.
- diffsToChanges, diffChange (type) — only used by handleGetDiff above.
- Removes now-unused imports `strconv` and `dmp` (sergi/go-diff).
internal/server/middleware_ratelimit.go
- writeTooManyRequests — never called; the live ratelimit middleware
uses a dedicated 429 path with Retry-After-Bucket headers.
internal/server/server.go
- guestVisibleItemIDs — never called. handlers_events.go had a
comment cross-reference; updated to drop the reference.
## Constants
internal/events/redis_bus.go
- reconnectDelay — never read.
internal/store/api_tokens.go
- defaultTokenExpiryDays — never read.
## Out of scope
The 5 unwired handlers in internal/server/handlers_documents.go are
left alone: they are the subject of TASK-769 (a product decision —
wire up vs. delete — that may want different treatment per handler).
The two SA4006/SA4010 findings on internal/server/handlers_dashboard.go
visibility-filter block are similarly left for TASK-765.
## Verified
- `go build ./...` clean
- `go vet ./...` clean
- `go test ./...` all pass
- `staticcheck -checks "SA*,U1000" ./...` clean except the two TASK-
765 / TASK-769 follow-ups noted above.
Parent: PLAN-644.
* docs: correct caller name in buildReconcileFindings doc (TASK-764)
Codex round 1 caught: the doc comment named the caller `reconcileSingle`
but the actual function is `reconcileItem` (cmd/pad/reconcile.go:204).
Fix the contract comment so it doesn't go stale on the first git blame.
|
||
|
|
157ca4e88f |
chore: bump Go toolchain to 1.26 (TASK-763) (#247)
* chore: bump Go toolchain to 1.26 (TASK-763) Bump Go from 1.25 to 1.26 across all toolchain pins: - go.mod — go 1.25.0 → go 1.26.0 - Dockerfile — golang:1.25-alpine → golang:1.26-alpine - .github/workflows/ci.yml — three setup-go steps (Go, Go-Postgres, E2E jobs) - .github/workflows/release.yml — release pipeline No `toolchain` directive: the repo is pre-launch with no external contributors yet, so we set the floor where we want it (hard requirement). Verified locally before commit: - golangci-lint v2.11.4 builds and runs under Go 1.26.2 (pinned in CI) - golang:1.26-alpine and 1.26.2-alpine images present on Docker Hub - go build ./... clean - go vet ./... clean - go test ./... all pass Parent: PLAN-644 (OSS Repo Hygiene and Launch Polish). * chore: gofmt -w under Go 1.26 (TASK-763) Apply Go 1.26's gofmt to the codebase. ~41 files reformatted, all struct-tag whitespace realignment — no semantic changes. Verified: - gofmt -l ./cmd ./internal returns empty after - go build ./... still clean - go test ./... still passes (run before commit) Bundling the gofmt diff with the toolchain bump in the same PR because the formatting drift is a direct consequence of moving from 1.25 to 1.26; splitting them creates a mandatory two-PR ordering for no value. Parent: PLAN-644. * docs: bump documented Go floor to 1.26 (TASK-763) Match go.mod's hard 1.26.0 requirement in the source-build instructions. Caught by Codex review round 1 on PR #247. - README.md:158 — "Go 1.25+" → "Go 1.26+" - CONTRIBUTING.md:9 — "Go 1.25+" → "Go 1.26+" |
||
|
|
6cda2da48d |
feat(billing): cancel Stripe customer on account delete (TASK-690) (#227)
* feat(billing): cancel Stripe customer on account delete (TASK-690) Parent: PLAN-645. Pair with pad-cloud PR #12. * fix(billing): abort on all non-200 per Codex review (round 1) * fix(billing): env wiring + docstrings + partial_delete test per Codex review (round 2) * fix(compose): wire cloud env vars from .env per Codex review (round 3) |
||
|
|
ac744fce2b |
fix(docs): replace 'pad serve' with 'pad server start' (TASK-675) (#199)
The 'pad serve' command does not exist in this binary — its canonical name has been 'pad server start' for some time. Users following the systemd example in docs/deployment.md would get a non-starting service today. Six real references fixed: - cmd/pad/main.go:5714 — migrate-to-pg help text - cmd/pad/main.go:5795 — 'Next steps' instruction - docs/backup.md:81,94 — Postgres migration walkthrough - docs/deployment.md:116 — binary launch example - docs/deployment.md:164 — systemd ExecStart Repo-wide grep is now clean of 'pad serve' outside gitignored v1-archive/ and .pad/ (local workspace data). README.md was already correct. Parent: PLAN-644. |
||
|
|
46fa72ca0f |
feat(server): log session IP changes, add optional PAD_IP_CHANGE_ENFORCE=strict (TASK-666) (#191)
* feat(server): log session IP changes, add optional PAD_IP_CHANGE_ENFORCE=strict (TASK-666)
Sessions stored a client IP at creation but never rechecked it. A stolen
cookie could be used from anywhere with no signal to the owner. This
change adds mid-lifetime IP-change detection without breaking legitimate
mobility (mobile roaming, VPN toggles, carrier NAT) by default.
- New audit action ActionSessionIPChanged captures {old_ip, new_ip} in
the audit metadata. Visible via the existing /api/v1/admin/audit-log.
- handleSessionIPChange wired into both SessionAuth (cookies) and
TokenAuth (padsess_ bearer). After UA check passes, compares stored
session IP to clientIP(r). On mismatch:
- log one audit row
- update the stored session IP so we don't spam the log
- strict mode: DeleteSession + 401 "session_ip_changed"
- default mode: let the request through
- Store.UpdateSessionIP lets middleware refresh the recorded IP without
tearing down the session.
- PAD_IP_CHANGE_ENFORCE=strict env var + ip_change_enforce TOML key +
Server.SetIPChangeEnforce setter (case-insensitive, trims whitespace).
- Table-driven tests cover log-only, strict rejection with session
destruction, and setter parsing edge cases.
Parent: PLAN-643 (OSS Security Hardening).
* fix(server): dedupe session-IP-change audit via CAS, handle browser vs API paths per Codex review
Addresses two P2 comments on PR #191:
1. Race: parallel requests after an IP change could each emit
ActionSessionIPChanged before any of them updated the stored IP,
producing duplicate audit rows for a single transition.
- Replace UpdateSessionIP with UpdateSessionIPIfEquals (compare-and-set
on ip_address). Only the request that actually rotates the stored
value logs; concurrent siblings lose the CAS and skip logging.
- New test TestSessionIPChange_CASDedupesRace fires 20 concurrent
requests from the new IP and asserts exactly 1 audit row.
2. Strict-mode 401 on non-API paths:
- In current routing the SPA is mounted on the root router outside
the auth Group, so SessionAuth only fires for /api/* in practice.
The original concern about JSON 401s on browser navigation doesn't
surface today, but defense-in-depth keeps the code forward-safe:
restructure handleSessionIPChange to return a four-state outcome
(Continue / AllowedLogged / Revoked / Terminated) and only write
the JSON 401 on /api/* paths. Revoked + non-API falls through
unauthenticated so a future SPA-in-group configuration would still
render a login screen instead of raw JSON.
- Clear the session cookie (MaxAge=-1) in strict rejection so the
browser stops sending the now-revoked token on the next request.
TestSessionIPChange_StrictClearsCookies verifies the Set-Cookie.
Parent: PLAN-643 (OSS Security Hardening), TASK-666.
* fix(server): strict mode destroys session atomically, never rotate stored IP when destroying (TASK-666)
Addresses Codex P1 on PR #191: previously we rotated the session's stored
ip_address via UpdateSessionIPIfEquals BEFORE attempting DeleteSession.
If the DELETE failed (transient DB error) the row remained alive —
rebound to the attacker's new IP — so follow-up requests saw stored IP
== client IP and passed handleSessionIPChange's "match, no-op" branch.
That silently defeated strict enforcement.
- New Store.DeleteSessionIfExists returns (bool, error) to serve as the
CAS primitive for strict mode: only the caller whose DELETE affected a
row emits the audit entry, and a DB error fails closed (500 — "Unable
to validate session") rather than letting the request through.
- handleSessionIPChange splits into two paths:
* log-only mode: UpdateSessionIPIfEquals for CAS dedup (unchanged)
* strict mode: DeleteSessionIfExists is the CAS; stored IP is NEVER
rotated so any failure leaves the session bound to the OLD IP and
subsequent requests from the new IP still mismatch + still reject.
- TestSessionIPChange_StrictDestroysSessionAtomically regression test
verifies a second request from the new IP with the same token still
fails after the first strict-mode rejection.
Parent: PLAN-643 (OSS Security Hardening).
* fix(server): exempt public API paths from strict IP-change termination (TASK-666)
Addresses Codex P2 on PR #191: SessionAuth runs for every /api/* path,
including public endpoints like /api/v1/auth/login, /api/v1/auth/register,
/api/v1/health, /api/v1/s/* (share links), and /api/v1/plan-limits. In
strict mode, a stale session cookie on those requests was rejected with
a 401 session_ip_changed BEFORE the public handler could run — the user
literally couldn't log back in because their own stale cookie blocked
the login call.
- Extract isPublicAPIPath as a shared helper between RequireAuth and
handleSessionIPChange so they can't drift out of sync.
- handleSessionIPChange strict-mode flow now: destroy session + clear
cookies + audit log (unchanged), then for public API paths return
Revoked so the handler still runs. For authenticated-only API paths
still return Terminated (401). For non-API paths return Revoked for
the SPA fallback.
- Updated TokenAuth Revoked handler to match: pass through unauth on
public paths, 401 on authenticated-only.
- TestSessionIPChange_StrictAllowsPublicAPIPaths regression test:
a stale session cookie on /api/v1/auth/login must NOT produce
session_ip_changed; /api/v1/plan-limits must still return 200.
Parent: PLAN-643 (OSS Security Hardening).
* fix(server): short-circuit SessionAuth on token auth + fix IPv6 clientIP parsing (TASK-666)
Addresses two more Codex comments on PR #191:
P1 — SessionAuth 401'd API-token-authenticated requests:
TokenAuth sets currentUser for user-owned tokens AND tokenWorkspaceID
for legacy workspace-scoped tokens. SessionAuth short-circuited only on
currentUser, so a workspace-scoped-token request that happened to carry
a stale session cookie with a mismatched IP would be rejected by the
IP-change strict path before RequireAuth could honor the token. Extend
the short-circuit to also check tokenWorkspaceID; either signal is
enough to say "token auth already succeeded, skip cookie validation".
P2 — clientIP mangled IPv6 addresses:
clientIP used strings.LastIndex(":") on RemoteAddr. For bare IPv6
addresses like "2001:db8::1" (which TrustedProxyRealIP writes verbatim
from X-Forwarded-For, no brackets/port), that strips the final hextet
to "2001:db8:" — unusable for comparison in the new IP-change audit
path and incorrect for rate-limit keys too. Switch to net.SplitHostPort
which handles both "host:port" and "[ipv6]:port", falling back to the
raw RemoteAddr when no port is present (the trusted-proxy rewrite
case).
Tests:
- TestClientIP_IPv6NotMangled covers IPv4 w/wo port, bracketed IPv6,
bare IPv6 (no port, no brackets), and loopback forms.
- TestSessionAuth_ShortCircuitsOnAPITokenAuth exercises the worst case:
strict mode + valid API token + stale session cookie + new client IP.
Request must succeed (token wins) and NO new session_ip_changed audit
row must appear.
Parent: PLAN-643 (OSS Security Hardening).
* fix(server): canonicalize IPs before session-IP-change comparison (TASK-666)
Addresses Codex P2 on PR #191: raw-string comparison of session.IPAddress
vs clientIP(r) would fire session_ip_changed spuriously when the same
IPv6 address arrived in different valid textual representations (the
trusted-proxy path writes X-Forwarded-For verbatim, and different hops
normalize differently — "2001:0db8::1" vs "2001:db8::1" etc.).
- canonicalIP helper: net.ParseIP + stringify to collapse equivalent
IPv6 forms (compressed vs expanded, case, leading zeros) and IPv4-in-
IPv6 into a single canonical string. Non-parseable inputs pass through
unchanged so debug/malformed values behave predictably.
- handleSessionIPChange compares and logs the canonical forms. The CAS
still passes session.IPAddress (the raw stored value) to the DB — the
compare-and-set is about row identity — but the new IP written in is
the canonical form so future comparisons are stable.
- TestCanonicalIP covers empty, IPv4, shorthand "::1", expanded 8-group
equivalent, mixed-case 2001:DB8::1, fully expanded 2001:0db8:…:0001,
and non-IP fallback.
Parent: PLAN-643 (OSS Security Hardening).
|
||
|
|
6f468d37b6 |
fix(config): auto-generate PAD_ENCRYPTION_KEY on first run (TASK-668) (#189)
* fix(config): auto-generate PAD_ENCRYPTION_KEY on first run (TASK-668)
store/encryption.go silently accepted an empty key and stored TOTP
seeds in plaintext; cmd/pad/main.go only logged a WARN. Operators who
never saw the warning (or saw it and ignored it) ran for months with
sensitive data at rest in the clear.
Change: encryption is now mandatory. Resolution order inside
Config.EnsureEncryptionKey:
1. PAD_ENCRYPTION_KEY env var (EncryptionKeySource = "env").
2. encryption_key in config.toml (source = "config").
3. <DataDir>/encryption.key file (source = "file").
4. Generate a fresh 32-byte AES-256 key, persist it to the file
above with 0600 permissions, continue (source = "generated").
Generation step never fails silently — mkdir + write errors propagate
out of main.go and abort startup.
main.go:
- drop the "if cfg.EncryptionKey != '' { enable } else { warn }" fork.
- call cfg.EnsureEncryptionKey(), fail startup on error, log at WARN
when a key is freshly generated so operators notice the new file.
Tests (internal/config/encryption_key_test.go):
- generates when missing (file permissions 0600, 32-byte key).
- loads existing file (strips trailing newline).
- respects already-configured values (no file write).
- idempotent across restarts (same key across two Config objects
sharing a DataDir).
Parent: PLAN-643 (OSS Security Hardening).
* fix(config): refuse to auto-generate key in clustered deployments per Codex P1
Codex caught that auto-generating a per-process key on a Postgres-
backed multi-replica deployment would give each replica its own key —
cross-instance decryption of shared DB rows would fail with GCM auth
errors.
Change: EnsureEncryptionKey now takes an allowGenerate bool. main.go
passes (dbDriver != 'postgres'): single-instance SQLite deployments
get the zero-config auto-generation path; Postgres deployments must
set PAD_ENCRYPTION_KEY explicitly. Operators who DO share a volume
across replicas can pre-seed the file and it still loads (the
generate step is the only thing gated).
Tests:
- TestEnsureEncryptionKey_RefusesToGenerateWhenClustered — allowGenerate=false
+ no existing file → error, no file written.
- TestEnsureEncryptionKey_ClusteredWithPreSeededFileStillLoads — the
file path works in clustered mode when the file is already present.
- Existing idempotency test updated to exercise the mixed case (first
boot generates, second boot loads with allowGenerate=false).
* fix(config): atomic encryption key file creation per Codex P2
Codex caught that the check-then-write sequence for encryption.key had
a race: two processes starting together could both pass the os.ReadFile
IsNotExist check, generate different keys, and race the write.
Whichever process wrote first would end up with an in-memory key that
no longer matched the persisted file, and future restarts of THAT
process would decrypt with the 'wrong' key.
Switch to os.OpenFile with O_CREATE|O_EXCL: on EEXIST we re-read the
file and converge on whichever key won the race. Every racing process
ends up with the same key or a clear startup error.
Test: TestEnsureEncryptionKey_ConcurrentStartIsRaceSafe fires 16
goroutines at a shared DataDir and asserts they all observe the same
key. Also runs clean under -race.
* fix(config): fully-written key guaranteed via temp+hardlink per Codex P2
Codex caught that O_CREATE|O_EXCL + ReadFile-on-EEXIST still had a
window where a loser could read an empty/partial file between the
winner's create and its first write. Hex/length validation would then
fail startup with a confusing error.
Switch to temp-file + os.Link:
1. Write the full key to a uniquely-named temp file (fully closed).
2. os.Link(temp, keyPath) atomically creates the final file as a
hardlink to the complete temp inode. EEXIST means a loser; the
file they'd read is another process's already-complete temp.
3. defer os.Remove(tmpPath) cleans up in every path.
The race-safety test now also covers the 'read partial' case
implicitly — if any goroutine loaded an empty/partial key the hex
decode in main.go would fail in production; the test asserts all 16
goroutines observe the same non-empty key.
* fix(config): reject world/group-readable encryption.key per Codex P2
Codex flagged that the file-load path blindly accepted any mode on
encryption.key. On a multi-user host, a pre-seeded file chmod'd to
0644 would hand the AES key to every local user, defeating the whole
purpose of encrypting TOTP seeds at rest.
Stat the file and reject any mode where group or other bits are set
(0077 mask). Error message points the operator at the fix (chmod 600).
Skipped on Windows where Unix permission bits aren't enforced.
Test: TestEnsureEncryptionKey_RejectsWorldReadableFile pre-seeds the
file at 0644 and verifies startup fails with the chmod hint.
* fix(config): always allow key auto-gen; warn on Postgres per Codex P1
Codex caught that gating auto-generation on 'not postgres' broke the
first-boot experience for every Postgres deployment that wasn't already
provisioning PAD_ENCRYPTION_KEY — which includes our own
docker-compose.yml and deploy/k8s/configmap.yaml. Server would exit
with 'encryption key required' before even starting.
Revert the gate: EnsureEncryptionKey(true) always, for every driver.
In exchange, log a WARN specifically on Postgres when we generate a
key, pointing operators at the multi-replica concern.
Trade-off accepted: single-instance Postgres just works; multi-replica
operators get a visible warning and clear failure mode (GCM auth
errors on first cross-replica read) if they don't act on it. Better
than a startup crash for the single-replica majority.
* fix(config): Postgres requires explicit PAD_ENCRYPTION_KEY; provision it in deployments
Codex was right twice — both concerns are real, and this commit
resolves them together:
1. Restore the Postgres gate: EnsureEncryptionKey(false) when
dbDriver == "postgres". Multi-replica deployments must share a
key; auto-generating per pod would fail cross-replica decryption.
2. Update the shipped Postgres deployments to provision a shared
PAD_ENCRYPTION_KEY so first-boot works out of the box:
- docker-compose.yml: PAD_ENCRYPTION_KEY via ${VAR:?err} shell
substitution (fails "docker compose up" with a clear message
if missing, matching the POSTGRES_PASSWORD pattern).
- .env.example: document PAD_ENCRYPTION_KEY as REQUIRED on
Postgres with an "openssl rand -hex 32" hint.
- deploy/k8s/secret.yaml: add PAD_ENCRYPTION_KEY with a
CHANGE_ME placeholder, explain why the replicas: 2 deployment
requires a shared key.
SQLite deployments continue to auto-generate on first boot (the
TASK-668 happy path), so single-user installs stay zero-config.
|
||
|
|
7d3b468fc8 |
feat(server): gate /metrics behind loopback + bearer token (TASK-653) (#180)
cmd/pad/main.go:277 unconditionally registered Prometheus metrics and internal/server/server.go:229 served /metrics with no auth/CSRF. Any caller on the network could read workspace counts, API usage patterns, and (via label enumeration) user/workspace IDs. Three-layer gate: 1. Loopback-only default. No PAD_METRICS_TOKEN configured → /metrics accepts loopback peers only (safe for self-hosters running Prometheus on the same host, which is the common case). Non-loopback peers get 403 with a clear message. 2. Bearer-token mode. PAD_METRICS_TOKEN set → every scrape must send "Authorization: Bearer <token>", compared in constant time. Missing or wrong header → 401 with WWW-Authenticate: Bearer realm="metrics". 3. Rate-limit/logging chain still wraps the endpoint from the outer router.Use calls. Wiring: - internal/config/config.go — MetricsToken field + PAD_METRICS_TOKEN env. - cmd/pad/main.go — plumb cfg.MetricsToken into SetMetricsToken. - .env.example — document PAD_METRICS_TOKEN with openssl-rand hint. - internal/server/server.go — metricsAuth middleware + subtle.ConstantTimeCompare. Tests: metrics_auth_test.go covers loopback allowed, LAN denied, missing/wrong/correct Bearer, non-Bearer scheme rejected, WWW-Authenticate header, and the SetMetrics-absent 404. Parent: PLAN-643 (OSS Security Hardening). |
||
|
|
5fffee2b9e |
fix(docker): bind to 127.0.0.1 + require POSTGRES_PASSWORD (TASK-661) (#174)
* fix(docker): bind to 127.0.0.1 + require POSTGRES_PASSWORD (TASK-661)
A fresh Docker install previously published 7777 on 0.0.0.0 with a
hardcoded pad:pad Postgres credential. The bootstrap endpoint is
reachable until the first admin is created, so this combination lets
anyone who can route to the host claim the instance — and with M5's
X-Forwarded-For spoof (fixed in TASK-660) chained with the loopback
bootstrap check, it became a full takeover.
Changes:
- docker-compose.yml: publish "127.0.0.1:7777:7777" by default, with a
PAD_BIND_ADDR override for operators who intentionally want LAN
access. Require POSTGRES_PASSWORD via ${VAR:?err} so docker compose
refuses to start when it's unset — can't silently inherit a weak
default credential.
- docker-compose.prod.yml: drop the "change-me-in-production"
placeholder; require the same env var as the base file.
- .env.example: new file documenting POSTGRES_PASSWORD (required),
PAD_BIND_ADDR, REDIS_PASSWORD, PAD_CLOUD_SECRET, PAD_ENCRYPTION_KEY,
PAD_TRUSTED_PROXIES with generation instructions.
- README.md: add a Docker Compose section covering the .env workflow
and the loopback-default → LAN override.
Parent: PLAN-643 (OSS Security Hardening).
* fix(docker): use libpq keyword=value DSN to avoid URI-encoding the Postgres password per Codex P1
Passwords produced by 'openssl rand -base64' often include '/', '+', or ':'
which are reserved in URI userinfo. Injecting them into postgres://user:PASS@...
breaks sql.Open. Switch PAD_DATABASE_URL to the libpq keyword=value form
(host=... password=... dbname=...) where the password is parsed as a single
token regardless of special characters.
Also teach pgDbnameFromURL to parse both DSN shapes so 'pad db backup/restore'
still shows the correct database name in its confirmation prompt.
|
||
|
|
ec9edef68c |
fix(server): gate RealIP on PAD_TRUSTED_PROXIES (TASK-660) (#173)
Replace the unconditional chimiddleware.RealIP with a middleware that only trusts X-Real-IP / X-Forwarded-For when the direct TCP peer is within a configured CIDR. With the safe default (PAD_TRUSTED_PROXIES unset) proxy headers are ignored entirely — the real TCP peer address is used for rate limiting, the bootstrap loopback check, and audit logs. Why: previously any client could set X-Forwarded-For to bypass per-IP rate limits AND the bootstrap loopback check (handlers_auth.go). On a direct-exposed Docker deploy (see M6, TASK-661) this compounded into a full-takeover chain. Gating RealIP breaks that chain even when the operator forgets to firewall the port. - internal/server/middleware_realip.go — new TrustedProxyRealIP middleware + ParseTrustedProxyCIDRs helper (accepts CIDRs or bare IPs, invalid entries logged+skipped, empty = nil result = no-op middleware). - internal/server/server.go — swap chimiddleware.RealIP for the gated version; add trustedProxyCIDRs field and SetTrustedProxies wiring. - internal/config/config.go — TrustedProxies field + PAD_TRUSTED_PROXIES env var. - cmd/pad/main.go — plumb config to the server. - internal/server/middleware_realip_test.go — covers no-trust default, untrusted peer, trusted peer with X-Real-IP, X-Forwarded-For first entry, and invalid header. Parent: PLAN-643 (OSS Security Hardening). |
||
|
|
e0f3583333 |
feat(cli): categorized template picker + interactive select (TASK-616) (#148)
* feat(cli): categorized template picker + interactive select (TASK-616)
Turns the CLI template picker from a flat alphabetical dump into a
category-aware flow that reflects the Software / People / Research /
Content / Operations / Personal taxonomy established by PLAN-609.
Library
-------
- collections.GroupTemplatesByCategory returns visible templates
bucketed into CategoryOrder with a trailing slot for any
custom-category templates — one canonical grouping that both CLI
and the upcoming web picker (TASK-617) can consume.
- collections.CategoryLabel turns category slugs into display labels
("software" → "Software") with passthrough for unknown values.
- collections.CategoryOrder exposes the canonical display order.
CLI
---
- New cmd/pad/templates_picker.go defines:
- printGroupedTemplates: writes the grouped listing with icons,
aligned columns, and a dim "(default)" marker on startup.
- pickTemplateInteractive: prompts when the user hasn't passed
--template and is on a TTY. Accepts a number OR a template name;
enter selects the default (startup); invalid input re-prompts.
- canPromptForTemplate: TTY detection so scripts never block.
- pad workspace init --list-templates now uses the grouped printer.
- pad workspace init / pad init error messages for unknown templates
show the grouped list instead of a flat dump.
- pad init now triggers pickTemplateInteractive when no --template
flag is set AND stdin/stdout are TTYs. Non-TTY invocations fall
back to the "startup" default unchanged.
- --template flag help no longer hardcodes "startup, scrum, product"
since the list now grows with non-software templates.
Tests
-----
- Library: TestGroupTemplatesByCategory (canonical order, no hidden,
every visible template assigned), TestCategoryLabel.
- CLI: TestPickTemplateInteractiveDefault / ByName / ByNumber /
RetriesOnInvalid, TestPrintGroupedTemplatesIncludesEveryVisibleTemplate
(smoke: every visible template renders, demo hidden, category
headers present).
Parent: PLAN-609.
* fix(cli): propagate non-EOF prompt read errors in template picker
Per Codex review on PR #148. pickTemplateInteractive previously
mapped any ReadString error to silently selecting the default
template. A detached PTY returning EIO or a similar read failure
would quietly create a workspace with the startup template even
though the user never made a valid choice. Restrict the silent
fallback to io.EOF (which is benign for pipes, tests, closed
stdin) and bubble up any other error so the command aborts.
* test(cli): cover non-EOF read error path in template picker
Adds TestPickTemplateInteractiveSurfacesNonEOFReadErrors to verify
the behavior change from the previous commit — a non-EOF read
failure propagates up instead of silently selecting the default
template.
|
||
|
|
115b33849e |
feat(templates): software starter pack + idempotent seeding (TASK-612) (#144)
* feat(templates): software starter pack + idempotent seeding (TASK-612) Ship the software templates (startup, scrum, product) with a curated starter pack of conventions + playbooks so new workspaces feel "batteries included" rather than empty shells. The pack is a safe, small subset drawn from the existing convention/playbook library — the library itself remains the full catalog for interactive onboarding. Starter pack contents --------------------- Conventions (4): - Conventional commit format (on-commit, should) - Never push directly to main (on-commit, must) - Run tests before completing tasks (on-task-complete, must) - Review your own changes before PR (on-pr-create, should) Playbooks (2): - Implementation Workflow (on-implement) - Code Review Process (on-review) The pack is materialized by looking up library items by title and converting them to SeedConvention / SeedPlaybook via json.Marshal of the expected field shape. When the library's wording changes, the template's seed content changes automatically. Store-side changes ------------------ SeedCollectionsFromTemplate is now idempotent with respect to seed items: items are only created in collections that were freshly created during the current call (tracked via a freshlyCreated set). That's the invariant that lets the server's startup auto-upgrade safely re-run on every boot without duplicating items across every workspace in the DB. Empty template name preserves the old behavior (default collections, no starter pack) — this keeps backward compatibility for callers that don't pass a template, including the server-startup auto-upgrade path and all existing server tests. Explicit "startup" / "scrum" / "product" now gets the starter pack. Tests ----- - TestSoftwareStarterPacksPopulated — guards against library-title drift - TestSoftwareTemplatesShipStarterPacks — each software template ships a pack - TestSeedCollectionsFromTemplateSeedsStarterPack — end-to-end seeding works - TestSeedCollectionsFromTemplateIdempotentWithSeedItems — re-seed doesn't duplicate Parent: PLAN-609. * fix(cli): default pad init to startup template when --template is omitted Per Codex review on PR #144. Without this, `pad workspace init` without `--template` no longer seeded the starter pack, even though startup is documented as the default. The fix lives in ensureWorkspace (shared by both init.go and the workspace creation command in main.go) — empty flag is rewritten to "startup" there. Tests and other direct API callers that want an empty workspace still pass Template="" through. * fix(cloud): auto-create workspace passes startup template for starter pack Per Codex review iteration 2 on PR #144. The auto-create cloud-signup flow calls SeedCollectionsFromTemplate with an empty template, which after this PR's semantics meant new cloud workspaces got no starter conventions/playbooks. Pass "startup" explicitly to match the CLI init behavior. * fix(store): propagate collection lookup errors during seeding Per Codex review iteration 3 on PR #144. seedItem previously treated any error from GetCollectionBySlug as a silent no-op, which hid real DB lookup failures — a transient error during workspace creation would make seeding appear successful while conventions/playbooks were in fact missing. Now we distinguish the two cases: - err != nil → propagate so callers can detect partial init - coll == nil → benign (template references a slug not in its collections list; template-author bug, no-op) * fix(store): idempotent seeding by item title (partial-init recovery) Per Codex review iteration 4 on PR #144. The previous design gated item seeding on collections being freshly-created-in-this-call, which trapped partially-initialized workspaces: if a DB error fired between collection creation and item seeding, a retry would see the collections already existed and skip every remaining seed item. Switch to title-based idempotency. Before inserting a seed item we list the target collection's existing items (once per collection, via a small cache) and skip any whose title already exists. That makes seeding: - Idempotent: re-running a template doesn't duplicate items - Recoverable: retrying fills in missing items after partial init - Retry-safe: the auto-upgrade path can re-run safely on every boot New test TestSeedCollectionsFromTemplateRecoversPartialInit exercises the recovery path explicitly. |
||
|
|
bf7901ab29 |
feat: add facet summary to CLI search output (#128)
Show collection breakdown (e.g. "docs: 9, ideas: 8, tasks: 53") after the result count when searching across all collections. Hidden when filtering by a specific collection. |
||
|
|
999bd3cfca |
feat: add pagination and sorting to search API (#123)
* feat: add pagination and sorting to search API Extend the search endpoint with limit/offset pagination and sort options. The response now includes total count (from a separate count query) so frontends can paginate properly. - Add Limit, Offset, Sort, Order to SearchParams with Normalize() defaults - Return SearchResponse struct with total/limit/offset metadata - Count query runs alongside results query for accurate totals - Sort options: relevance (default), created_at, updated_at, title - Add --sort, --limit, --offset flags to CLI search command - Update frontend SearchFilters and SearchResponse types - Add TestSearchPagination and TestSearchSorting integration tests * fix: count ref hits in search totals and handle empty pages - Ensure total is never less than actual results when direct ref matches (e.g. "TASK-5") aren't captured by the FTS count query - Handle empty page in CLI output: show "No results on this page" instead of an invalid descending range like "Showing 11-10 of 5" Addresses codex review on PR #123. * fix: paginate ref hits correctly and add sort tie-breaker - Ref hits now occupy slots on page 0 only; FTS limit/offset adjusted so combined results respect the requested pagination contract - On subsequent pages, ref hits are excluded (already shown on page 0) - Add i.id as deterministic tie-breaker to all ORDER BY clauses to prevent duplicate/missing items across paginated pages Addresses codex review on PR #123. |
||
|
|
aef0e2326a |
feat: add collection and field filtering to search API (#122)
* feat: add collection and field filtering to search API Extend the /search endpoint to support scoping by collection slug and filtering by structured field values (status, priority, and generic field.* params). Works on both SQLite FTS5 and PostgreSQL tsvector. - Add Collection and FieldFilters to SearchParams (store layer) - Parse collection, status, priority, field.* query params (handler) - Add SearchFilters type and update api.search() signature (frontend) - Add --collection, --status, --priority flags to CLI search command - Add integration tests for collection, field, and combined filtering * fix: validate field filter keys to prevent SQL injection Reject field filter keys containing special characters before they reach JSONExtractText, which interpolates keys directly into SQL. Keys must match ^[a-zA-Z][a-zA-Z0-9_-]*$ — validation is applied in both the handler and the store layer as defense in depth. Addresses codex review on PR #122. |
||
|
|
9072e49b17 |
feat: add CLI commands for item starring (#120)
Add star/unstar/starred CLI commands (PLAN-564, TASK-570): - pad item star <ref> — star an item - pad item unstar <ref> — unstar an item - pad item starred [--all] [--format json] — list starred items Client methods: StarItem, UnstarItem, ListStarredItems. |
||
|
|
2e03222aa5 |
feat: restore pad init as smart multi-step entry point (#99)
* feat: restore `pad init` as smart multi-step entry point Adds a top-level `pad init` command that detects the current state and walks through each setup step as needed: configure connection, start server, bootstrap admin account, authenticate, create/link workspace, and install/update AI skill files. Safe to re-run anytime — skips completed steps and shows a status summary when everything is healthy. Also refactors `pad workspace init` to use shared helpers and adds a hint pointing users to `pad init` when prerequisites are missing. Ref: IDEA-499, PLAN-546 * fix: validate --template flag before workspace creation in pad init Adds the same preflight template validation that workspace init has, preventing a partially initialized workspace from being created when an invalid template name is passed. Ref: PR #99 review feedback |
||
|
|
40e6a8b705 |
fix: make pad db backup and pad db restore work with SQLite
The db commands were PostgreSQL-only, which made them useless for self-hosted users on the default SQLite setup. Now both commands auto-detect the database driver and do the right thing: - SQLite (default): file copy of ~/.pad/pad.db with WAL/SHM handling - PostgreSQL: existing pg_dump/psql behavior (unchanged) |
||
|
|
7ca0463e70 |
feat: browser-based CLI authentication flow (#97)
Replace the email/password terminal prompt in `pad auth login` with a
browser-based auth flow. The CLI creates a pending session, prints a URL
the user opens in their browser (works for localhost, remote VPS, or
Pad Cloud), and polls until the session is approved.
- Add CLI auth session endpoints (create, poll, approve)
- Add browser approval page at /auth/cli/{code}
- Rewrite `pad auth login` to use browser flow by default
- Keep `pad auth login --interactive` as email/password fallback
- Add login page redirect param support for post-login bounce-back
- Add SQLite and PostgreSQL migrations for cli_auth_sessions table
Closes PLAN-539, IDEA-404
|
||
|
|
1ba9c91992 |
feat: email unsubscribe for non-transactional emails (#96)
* feat: email unsubscribe for non-transactional emails Add CAN-SPAM compliant unsubscribe support: - New email_optouts table (by email address, not user ID) so uninvited recipients can opt out without an account - HMAC-signed unsubscribe tokens (derived from Maileroo API key) so links work without authentication - GET /api/v1/unsubscribe endpoint with simple HTML confirmation page - Invitation emails now include unsubscribe footer link - Welcome emails accept unsubscribe URL parameter - Before sending invitation emails, check opt-out table and silently skip opted-out addresses (prevents invite spam) - Password reset emails are exempt (transactional, user-initiated) Fixes BUG-256. * fix: hide "Copy invite link" when code is unrecoverable For hashed invitations the plaintext code can't be recovered, so the button was copying a broken URL. Now shows "Sent via email" label instead. Only shows the copy button when join_url or code is available. Fixes BUG-255. |
||
|
|
d0518216c5 |
feat: add cloud infrastructure for hosted Pad (PLAN-427)
Add the foundation for running Pad as a hosted service at app.getpad.dev. Same binary in cloud mode with a thin sidecar for OAuth and Stripe. Cloud mode (PAD_CLOUD=true): - PAD_CLOUD flag with cloud secret for sidecar communication - Account-level billing: plan field on users, CheckLimit enforcement - Free/Pro tiers with configurable limits stored in platform_settings - Three-tier limit resolution: user overrides → DB defaults → hardcoded fallback - Plan enforcement on workspace, item, member, webhook, and token creation Authentication & security: - OAuth login endpoint (POST /api/v1/auth/oauth-login) with cloud secret gate - Verified email requirement for OAuth, 2FA bypass protection - Cloud secret rotation support (comma-separated keys) - TOTP secret encryption at rest (AES-256-GCM via PAD_ENCRYPTION_KEY) - Rate limiting on OAuth login endpoint - Bootstrap disabled in cloud mode - Password max length enforcement (128 chars) - Config file written with 0600 permissions Admin & billing: - Admin user management API (list, detail, update plan/overrides) - Configurable plan limits API (GET/PATCH /api/v1/admin/limits) - Platform stats endpoint - Admin plan endpoint for sidecar to set user plans - GDPR: account deletion and data export endpoints Console UI (cloud mode only): - /console — workspace list with owned/shared sections - /console/new — create workspace wizard with slug preview - /console/settings — profile, password, API tokens - /console/billing — plan status, upgrade/manage links - /console/admin — user management, plan overrides, limits editor - OAuth buttons (GitHub/Google) on login page in cloud mode Auto-create default workspace on signup in cloud mode. Migration 035: plan, plan_expires_at, stripe_customer_id, plan_overrides on users. |
||
|
|
1cbd7ba204 |
fix: resolve workspace routing regressions from username refactor
- Fix UUID-shaped workspace slugs: resolveWorkspace now falls back to slug-based lookup when a UUID doesn't match any workspace ID - Fix imported workspaces: ImportWorkspace accepts ownerID, handler sets authenticated user as owner and adds workspace membership - Fix generated username collisions: add EnsureUniqueUsername to append suffixes (-2, -3, etc.) when auto-generated usernames already exist - Fix handler-level UUID resolution: workspace CRUD handlers now use getWorkspace helper (reads middleware-resolved ID from context) instead of raw URL params with slug-only store methods |
||
|
|
10867b9210 |
fix: persist 2FA challenge key and prevent duplicate TOTP verification
- Persist the 2FA challenge HMAC signing key in platform_settings so tokens survive process restarts and work across multiple instances - Add AND totp_enabled = false to EnableTOTP WHERE clause so concurrent /auth/2fa/verify calls (double-click, multi-tab) cannot both succeed and overwrite each other's recovery codes |
||
|
|
5606b22007 |
fix: address 6 security findings from Codex review of TOTP 2FA
HIGH fixes: - Login-verify no longer accepts bare user_id. Now requires an HMAC-signed, IP-bound, 5-minute challenge token issued during login (prevents password bypass via known user ID + TOTP code) - Recovery codes are SHA-256 hashed before storage; plaintext is returned to the user once and never persisted MEDIUM fixes: - ConsumeRecoveryCode uses a DB transaction to prevent concurrent double-consumption of the same recovery code - EnableTOTP is atomic: WHERE clause requires totp_secret match to prevent TOCTOU race between setup and verify calls - /auth/2fa/login-verify now uses the strict Auth rate limiter (5 req/min/IP) instead of the general API limiter - CLI login detects requires_2fa response and prompts for TOTP code instead of silently saving empty credentials |
||
|
|
bde15d45ca |
Rename Phases to Plans, clean up deprecated aliases (#71)
* Rename "Phases" to "Plans" and clean up deprecated phase aliases
Renames the default "Phases" collection to "Plans" across the full stack:
- DB migration renames existing collections in-place (name, slug, prefix PLAN, icon 🗺️)
- Removes all deprecated Phase* backward-compat aliases from models and store
- Removes --phase CLI flag (use --parent instead)
- Updates convention triggers: on-phase-start/complete → on-plan-start/complete
- Updates dashboard API: active_phases → active_plans, /phases-progress → /plans-progress
- Updates all frontend components, types, and documentation
Closes IDEA-124
* Fix CSRF cookie not being cleared on logout
The SessionAuth middleware was re-issuing a CSRF cookie before the
logout handler could clear it, resulting in two Set-Cookie headers.
Skip CSRF re-issue for /api/v1/auth/ paths since auth endpoints
manage their own CSRF cookies (login sets, logout clears).
* Fix migration issues found in Codex review
- P1: Move doc_type UPDATE from migration 024 into 025, which recreates
the table with the new CHECK constraint first (SQLite enforces CHECK
on UPDATE, so the old constraint would reject 'plan')
- P1: Add PostgreSQL migration 005 for the collection rename (phases →
plans) — previously only existed on the SQLite path
- P2: Recreate FTS triggers, indexes, and rebuild FTS after the table
swap in migration 025 (DROP TABLE drops associated objects in SQLite)
* Fix parent filter field name and sync .agents skill copy
Codex review round 2 findings:
- P1: Parent filter compared against `parent_id` (wrong) instead of
`parent_link_id` — plan filtering in collection view was broken
- P1: .agents/skills/pad/SKILL.md still had old --phase flags and
"Phases" references — synced from the updated .claude copy
- P2: Accept legacy 'phase' filter key for backward compat with
existing saved views that serialized the old key name
* Fix PG migration JSONB casting and add slug collision guards
Codex PR review bot findings:
- P1: PostgreSQL REPLACE/LIKE don't work on JSONB columns — cast
schema::text and fields::text before string ops, then back to ::jsonb
- P1: If a workspace already has a custom 'plans' collection, the
rename hits UNIQUE(workspace_id, slug) — added NOT EXISTS guard
to both SQLite and PostgreSQL migrations
|
||
|
|
063ff92d00 |
feat: generalized parent/child items with progress tracking (#70)
* feat: generalize parent/child items — any item can have children with progress tracking
Replaces the phases-only task widget with a generalized parent/child system.
Any item (Phase, Idea, Doc, Task, etc.) can now be a parent of child items,
getting automatic progress bars, burndown charts, status grouping, drag-drop
reordering, and recursive expand/collapse up to 3 levels deep.
DB: migrate link_type 'phase' → 'parent' (migration 023)
Store: generalized methods (GetChildItems, GetItemProgress, SetParentLink
with cycle detection), drop collection filters, per-child terminal status
API: new /items/{slug}/children and /items/{slug}/progress endpoints
Frontend: ChildItems, ChildChart, NestedChildren components replace PhaseTasks
CLI: --parent flag (--phase kept as hidden alias), list/show/changelog updated
Docs: CLAUDE.md, SKILL.md, pad-web updated for parent/child model
Full backward compatibility: old 'phase' link_type, --phase flags, phase_id/
phase_ref JSON fields all still work as deprecated aliases.
Closes PHASE-16 (9 tasks).
* fix: update collection list page to use item_id from phasesProgress response
The TS client return type changed from phase_id to item_id but the
collection list page still referenced p.phase_id, causing svelte-check
type errors in CI.
* fix: address Codex review findings — PG migration, terminal statuses, metrics, CSRF resilience
- Add PostgreSQL migration 003 to rename 'phase' links to 'parent'
- Use schema-defined terminal statuses in GetAllItemProgress instead of hardcoded defaults
- Pass computed terminal statuses from page to ChildItems component
- Only special-case 'parent' field key when not defined in collection schema
- Move MetricsMiddleware before Recoverer so panics are counted
- Always mount ChildItems for SSE subscriptions even with 0 children
- Exclude soft-deleted children from has_children enrichment query
- Re-issue CSRF cookie when session is valid but cookie is missing
- Show actual API error messages in create-item toasts
|
||
|
|
34ebf31fdf |
fix: resolve Codex review findings across SSE, audit, metrics, and deployment
- Make SSE limit checks atomic with subscription via SubscribeIfAllowed to prevent TOCTOU races where concurrent requests bypass connection caps - Replace fmt.Sprintf JSON assembly with json.Marshal (auditMeta helper) in all audit log call sites to prevent silent JSONB insert failures on PostgreSQL when metadata contains special characters - Pass database credentials via PGDATABASE env var instead of pg_dump/psql command-line args to avoid leaking passwords in ps/proc output - Fix audit-log query builder to rebind placeholders once after all filters are appended, preventing duplicate $1 placeholders on PostgreSQL - Replace per-workspace SSE GaugeVec with a single Gauge to avoid unbounded Prometheus label cardinality in multi-tenant deployments - Fix prod Docker Compose: override PAD_REDIS_URL with password and add authenticated Redis healthcheck when REDIS_PASSWORD is set |
||
|
|
77d756c677 |
fix: address Codex review findings for backup and audit trail
P1 fixes: - Use --dbname=URL for pg_dump/psql so SSL params, timeouts, and other connection options from PAD_DATABASE_URL are preserved - Add --clean --if-exists to pg_dump so restores can overwrite an existing database without duplicate-key errors P2 fixes: - Add logAuditEventForUser() to pass explicit user ID for auth events (login, register, bootstrap, password_reset) where the request context doesn't yet have the authenticated user - Change audit log --actor filter to match user_id column instead of actor type, so filtering by specific user actually works |
||
|
|
1d26283752 |
feat: add PostgreSQL backup, restore, and migration CLI commands
- pad db backup: wraps pg_dump with --output and --cron flags - pad db restore: wraps psql with confirmation prompt and --force - pad db migrate-to-pg: one-time SQLite→PostgreSQL migration using application-level export/import for all workspace data - docs/backup.md: comprehensive backup strategy guide covering SQLite, PostgreSQL, cloud snapshots, and disaster recovery |
||
|
|
872f08aa84 |
feat: add compliance audit trail with IP/UA tracking
Extend the activities table to capture IP address and user agent for all state-changing operations. Add audit events for auth (login, logout, register, bootstrap, password changes), workspace management (member invite/remove, role changes), token lifecycle, and admin settings. - SQLite migration recreates activities table with nullable workspace_id, new ip_address/user_agent columns, and relaxed CHECK constraints - PostgreSQL migration adds columns and drops constraints - New ListAuditLog store method with action/actor/workspace/date filters - GET /api/v1/audit-log endpoint (admin-only) - CLI: pad workspace audit-log [--days N] [--actor X] [--action X] |
||
|
|
82e93d6014 |
feat: implement SSE connection limits
Add configurable global and per-workspace SSE connection limits to prevent memory exhaustion from unbounded connections. Returns HTTP 429 when limits are reached. Logs warnings at 80% capacity. Configurable via PAD_SSE_MAX_CONNECTIONS (default 1000) and PAD_SSE_MAX_PER_WORKSPACE (default 100), or config.toml. Adds WorkspaceSubscriberCount to EventBus interface for per-workspace tracking (MemoryBus iterates subscribers, RedisBus uses existing wsCounts map). Resolves TASK-165 |
||
|
|
20fbb45de9 |
feat: add Prometheus metrics and /metrics endpoint
Instrument the Go server with Prometheus metrics for production monitoring. Adds HTTP request count/duration/size histograms (by method, route pattern, status), SSE connection gauges per workspace, event bus publish/subscriber counts, and database connection pool stats via callback collector. Go runtime metrics included. The /metrics endpoint is unauthenticated (standard for Prometheus scraping), separated from the auth middleware via chi router groups. Resolves TASK-164 |
||
|
|
935b3d7b0e |
fix: PG agent role reorder, JSONB tag filters, Redis credential leak
- Rebind prepared statements in agent role and card reordering so PostgreSQL receives $1/$2 instead of ? placeholders - Add JSONArrayContains dialect method: SQLite uses LIKE, PostgreSQL uses jsonb @> operator — fixes tag filtering on JSONB columns - Redact Redis credentials from startup log: log addr+db only, not the full connection URL which may contain passwords |
||
|
|
63d5cb7b73 |
fix: address Codex review findings for PostgreSQL compatibility
P1: Wrap store helper queries (uniqueSlug, uniqueSlugExcluding, backfillItemNumbers) with s.q() for placeholder rebinding. P1: Replace boolToInt() with s.dialect.BoolToInt() so pgx receives native booleans instead of 0/1 integers. P1: Change boolean scan variables from int to bool to match PostgreSQL's native boolean type. P1: Fix FTS table aliases in PostgreSQL search branches. P1: Make api_tokens.workspace_id nullable for user-scoped tokens. P2: Move eventBus.Close() before srv.Shutdown() so SSE handlers drain before the HTTP server shutdown deadline. |
||
|
|
a4a701367a |
feat: add PostgreSQL support with dual-driver store layer (TASK-157)
- Create Dialect abstraction for SQLite/PostgreSQL SQL differences (JSON ops, FTS, placeholders, datetime, aggregation) - Add Store.NewPostgres() constructor with connection pooling - Create consolidated PostgreSQL schema (pgmigrations/001_initial.sql) with tsvector FTS, JSONB columns, and GIN indexes - Refactor all store queries (~150) to use s.q() for placeholder rebinding - Replace hardcoded json_extract/FTS5/GROUP_CONCAT with dialect methods - Support PAD_DB_DRIVER=postgres + PAD_DATABASE_URL env vars - Keep SQLite as the default for local/self-hosted mode - Add dialect unit tests (rebind, SQLite, PostgreSQL) |
||
|
|
b9d0a89195 |
feat: add Redis pub/sub EventBus for multi-instance SSE (TASK-158)
- Extract EventBus interface (Subscribe, Unsubscribe, Publish, Close) - Rename Bus → MemoryBus, keeping it as the default for single-instance - Add RedisBus implementation with per-workspace channel subscriptions - Lazy Redis subscribe/unsubscribe as SSE clients connect/disconnect - Configure via PAD_REDIS_URL env var; falls back to in-memory without it - Update Server.SetEventBus to accept the EventBus interface |
||
|
|
e7f4448028 |
feat: add readiness probe and structured logging (TASK-161)
- Add /health/live (liveness) and /health/ready (readiness with DB check) endpoints - Add Store.Ping() for database connectivity verification - Create internal/logging package using stdlib log/slog - Support PAD_LOG_LEVEL (debug/info/warn/error) and PAD_LOG_FORMAT (text/json) env vars - Add structured request logging middleware replacing chi's default Logger - Migrate all log.Printf calls to slog with proper levels and key-value attrs - Exempt health probe endpoints from auth middleware |
||
|
|
dab6d6c9c9 |
feat: implement graceful shutdown with request draining (TASK-159)
- Add signal handler for SIGINT/SIGTERM with 30s grace period - Add Server.Shutdown() for graceful HTTP connection draining - Add EventBus.Close() to cleanly terminate SSE subscribers - Configure HTTP server timeouts (read: 15s, header: 5s, idle: 120s) - Fix SetWebUI nil router panic by calling ensureRouter() - Add Server.Handler() for httptest compatibility |