mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-10 06:55:40 +00:00
main
43 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 dependabot[bot]
|
31d11e76ea |
chore(deps)(deps): bump github.com/mark3labs/mcp-go from 0.58.0 to 1.0.0 (#1275)
* chore(deps)(deps): bump github.com/mark3labs/mcp-go from 0.58.0 to 1.0.0 Bumps [github.com/mark3labs/mcp-go](https://github.com/mark3labs/mcp-go) from 0.58.0 to 1.0.0. - [Release notes](https://github.com/mark3labs/mcp-go/releases) - [Commits](https://github.com/mark3labs/mcp-go/compare/v0.58.0...v1.0.0) --- updated-dependencies: - dependency-name: github.com/mark3labs/mcp-go dependency-version: 1.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * fix(mcp): pad owns the protocol revision it advertises, not the library (TASK-2972) mcp-go 1.0 moves LATEST_PROTOCOL_VERSION to 2026-07-28. `MetaPayload. MCPProtocolVersion` was sourced from that constant, on the reasoning — written in the comment — that doing so meant the value "never drifts from what NewMCPServer actually advertises in the handshake". 1.0 falsified that, and in the direction the comment was guarding against. The handshake answers through `mcp.NegotiateLegacyVersion`, which returns at most LATEST_LEGACY_PROTOCOL_VERSION and CANNOT return the modern revision at all: measured, a client asking for 2026-07-28 is told 2025-11-25, and a client that sends nothing is told 2025-03-26. So the bump would have left the handshake where it was and moved the meta document alone — publishing a claim to negotiate a revision this server cannot negotiate. The advertised revision is now a pad-owned literal. Moving it means reading the new revision's delta against this server's surface first; a library bump must not move it on its own. The test that should have caught this was a tautology: it compared the payload against the same constant the payload was built from, so it could not fail, and it would have passed through this bump. Replaced with two assertions that each catch what the other cannot — against the LITERAL, so moving pad's claim is a deliberate edit visible in a diff, and against what the library's handshake ACTUALLY answers, which is the property the old comment claimed and never had. Both legs verified to fail when the constant is moved. Refs: TASK-2972 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: xarmian <xarmian@gmail.com> |
||
|
dependabot[bot]
|
b7235a1a61 |
chore(deps)(deps): bump the go-minor-and-patch group across 1 directory with 6 updates (#1301)
Bumps the go-minor-and-patch group with 6 updates in the / directory: | Package | From | To | | --- | --- | --- | | [github.com/alicebob/miniredis/v2](https://github.com/alicebob/miniredis) | `2.38.0` | `2.39.0` | | [github.com/prometheus/client_model](https://github.com/prometheus/client_model) | `0.6.2` | `0.6.3` | | [github.com/prometheus/common](https://github.com/prometheus/common) | `0.70.1` | `0.71.0` | | [golang.org/x/sys](https://github.com/golang/sys) | `0.47.0` | `0.48.0` | | [golang.org/x/time](https://github.com/golang/time) | `0.15.0` | `0.16.0` | | [modernc.org/sqlite](https://gitlab.com/cznic/sqlite) | `1.57.0` | `1.58.0` | Updates `github.com/alicebob/miniredis/v2` from 2.38.0 to 2.39.0 - [Release notes](https://github.com/alicebob/miniredis/releases) - [Changelog](https://github.com/alicebob/miniredis/blob/master/CHANGELOG.md) - [Commits](https://github.com/alicebob/miniredis/compare/v2.38.0...v2.39.0) Updates `github.com/prometheus/client_model` from 0.6.2 to 0.6.3 - [Release notes](https://github.com/prometheus/client_model/releases) - [Commits](https://github.com/prometheus/client_model/compare/v0.6.2...v0.6.3) Updates `github.com/prometheus/common` from 0.70.1 to 0.71.0 - [Release notes](https://github.com/prometheus/common/releases) - [Changelog](https://github.com/prometheus/common/blob/main/CHANGELOG.md) - [Commits](https://github.com/prometheus/common/compare/v0.70.1...v0.71.0) Updates `golang.org/x/sys` from 0.47.0 to 0.48.0 - [Commits](https://github.com/golang/sys/compare/v0.47.0...v0.48.0) Updates `golang.org/x/time` from 0.15.0 to 0.16.0 - [Commits](https://github.com/golang/time/compare/v0.15.0...v0.16.0) Updates `modernc.org/sqlite` from 1.57.0 to 1.58.0 - [Changelog](https://gitlab.com/cznic/sqlite/blob/master/CHANGELOG.md) - [Commits](https://gitlab.com/cznic/sqlite/compare/v1.57.0...v1.58.0) --- updated-dependencies: - dependency-name: github.com/alicebob/miniredis/v2 dependency-version: 2.39.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: github.com/prometheus/client_model dependency-version: 0.6.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-and-patch - dependency-name: github.com/prometheus/common dependency-version: 0.71.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: golang.org/x/sys dependency-version: 0.48.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: golang.org/x/time dependency-version: 0.16.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: modernc.org/sqlite dependency-version: 1.58.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
xarmian
|
4618876e3e |
fix(cli): pad server stop signals only a process it can prove is ours (BUG-2969) (#1299)
fix(cli): `pad server stop` signals only a process it can prove is ours (BUG-2969) Measured on the merged binary before this change: a `sleep 600` whose pid had been written into the PID file was SIGTERMed, and stop printed "Server stopped." No pad server was running anywhere near that config. Three things had to be true at once for that. os.FindProcess succeeds for ANY pid on Unix. Nothing asked whether the pid belonged to a pad server. And the confirmation loop polled the PORT — which is unhealthy from the first poll when nothing was ever serving, so the success check was satisfied by the failure case. Liveness is the wrong question, and this is the trap the obvious fix falls into: the stranger WAS alive. The question is whether the pid is OUR server. ## The discriminator Unix takes an advisory flock on the PID file, held for the server's lifetime. `stop` probes it non-blockingly: acquiring it proves nobody holds the file, so the record is stale whatever the pid now names; failing to acquire proves a live pad server holds THIS file. One implementation for Linux and macOS, no new dependency, and the same primitive session_lock_unix.go has used since TASK-2767. Windows has no flock in that pattern, so it compares the process creation time from GetProcessTimes against the one recorded at start — the attribute that survives pid reuse, since a reused pid belongs to a process that started later. The lead first ruled start-time comparison on every platform; I objected with the cost (three implementations — /proc, a macOS sysctl promoting x/sys to a direct dependency, and GetProcessTimes) and the ruling changed to this hybrid. The cost table is on the item so the next reader sees why the shape moved. The PID file gains a fingerprint on both platforms — pid, start time, executable path — as JSON, with the legacy bare-integer form still parsed. A legacy record carries no proof, which reads as UNPROVABLE, and unprovable means nothing is signalled. ## Three races, each found by codex and each the same shape 1. Reading the record and checking ownership were separate steps, so a successor could claim the file between them: the lock then reported "held" — truthfully, about the successor — while the pid handed back was the predecessor's. pidFileOwner now returns the record it read from the descriptor it probed. 2. Removing the PID file after a successful stop could delete a fast successor's live record. It no longer removes at all there: the server removes its own on the way down, and a file left by a crash is handled by the next stop. 3. Removing a STALE file after the probe released the lock had the same window. The removal now happens inside the ownership check, while the lock is held — the only moment at which no replacement can have claimed the path. A claim arriving during that instant retries for half a second rather than losing its claim for the life of the process. Windows deliberately does NOT delete a stale file: with no atomic primitive, a check-then-remove would race a successor, and a stale file that the next start overwrites is recoverable where a wrongly deleted record is not. ## Verified Negative control, and it is the literal one: with the ownership check bypassed, `go test` reports `signal: terminated` — the test binary is SIGTERMed by the code under test, because the stale record names the test process itself. Live, in throwaway HOMEs: a stale record naming a live `sleep` is refused and the sleep survives (it was killed before this change); a stale record with a HEALTHY port answering is still refused, nothing signalled, and both the stranger and the real server survive; a server stopped through its own held record stops, and its file is gone. The CI smoke on windows-latest now stops the server with `pad server stop` instead of Stop-Process, because that is the only place the Windows ownership check runs — a smoke that killed the process directly would leave the GetProcessTimes path unexercised on every platform. make lint, make test green; codex CLEAN in round 4. Claude-Session: https://claude.ai/code/session_01HeChkgZVYb3NTgTcckF5KR |
||
|
xarmian
|
0900be6241 |
chore(deps): bump golang.org/x/crypto to v0.56.0 (BUG-2851)
Two advisories published 2026-09-02 19:12Z (GO-2026-6354, GO-2026-6355; DoS in golang.org/x/crypto/ssh, fixed in v0.56.0) made govulncheck fail the Nix job on runners whose vulnerability database had them — intermittently across runners, not as a threshold: main at |
||
|
dependabot[bot]
|
ff969d23e0 |
chore(deps)(deps): bump the go-minor-and-patch group across 1 directory with 4 updates (#1180)
* chore(deps)(deps): bump the go-minor-and-patch group across 1 directory with 4 updates Bumps the go-minor-and-patch group with 4 updates in the / directory: [github.com/go-chi/chi/v5](https://github.com/go-chi/chi), [github.com/mark3labs/mcp-go](https://github.com/mark3labs/mcp-go), [golang.org/x/crypto](https://github.com/golang/crypto) and [modernc.org/sqlite](https://gitlab.com/cznic/sqlite). Updates `github.com/go-chi/chi/v5` from 5.3.1 to 5.3.2 - [Release notes](https://github.com/go-chi/chi/releases) - [Changelog](https://github.com/go-chi/chi/blob/master/CHANGELOG.md) - [Commits](https://github.com/go-chi/chi/compare/v5.3.1...v5.3.2) Updates `github.com/mark3labs/mcp-go` from 0.57.0 to 0.58.0 - [Release notes](https://github.com/mark3labs/mcp-go/releases) - [Commits](https://github.com/mark3labs/mcp-go/compare/v0.57.0...v0.58.0) Updates `golang.org/x/crypto` from 0.54.0 to 0.55.0 - [Commits](https://github.com/golang/crypto/compare/v0.54.0...v0.55.0) Updates `modernc.org/sqlite` from 1.56.0 to 1.57.0 - [Changelog](https://gitlab.com/cznic/sqlite/blob/master/CHANGELOG.md) - [Commits](https://gitlab.com/cznic/sqlite/compare/v1.56.0...v1.57.0) --- updated-dependencies: - dependency-name: github.com/go-chi/chi/v5 dependency-version: 5.3.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-and-patch - dependency-name: github.com/mark3labs/mcp-go dependency-version: 0.58.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: golang.org/x/crypto dependency-version: 0.55.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: modernc.org/sqlite dependency-version: 1.57.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch ... Signed-off-by: dependabot[bot] <support@github.com> * nix: vendorHash for the go-minor-and-patch bump The four module updates change the vendored dep set; hash taken from the fixed-output derivation mismatch on this PR's own Nix run (the in-branch fix the day-26 batch established on #1041). Claude-Session: https://claude.ai/code/session_01HvAuiZ7JaWyCqqyV99LyWt --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: xarmian <xarmian@gmail.com> |
||
|
xarmian
|
25c7cd20f5 |
feat(watchevents): Redis-backed bus so watch notifications cross instances (BUG-2651) (#1167)
* feat(watchevents): Redis-backed bus so watch notifications cross instances (BUG-2651) internal/watchevents shipped MemoryBus only, so in a multi-instance deployment a notification published on instance A never reached a stream held open on instance B — watches appeared to work and silently dropped. Bus was an interface from day one for exactly this; adding RedisBus changed no producer and no consumer. NOT A MECHANICAL PORT of internal/events.RedisBus. Three deliberate divergences, each documented at the point someone diffing the two files would call it a mistake: - ONE channel and ONE replay buffer, because this package has exactly one logical stream by contract (DOC-2479 DR-2: all per-caller filtering happens in the consumer). Most of the template's bookkeeping — per- workspace counts, subscriptions, buffers — has nothing to key on here. - EAGER subscription for the bus's lifetime, not lazily on first local subscriber. The replay buffer fills from the RECEIVE path, so a lazily torn-down subscription stops filling it at precisely the moment before a Last-Event-ID resume — for one harness monitor holding one stream, that makes resume structurally useless. The template can afford lazy because per-workspace means N idle subscriptions; here it is one. - ONE mutex across subscriber membership and the replay buffer, held through the whole local fan-out. The template uses two and offers only separate Subscribe + EventsSince, which cannot provide SubscribeAndReplaySince's guarantee. Copying its locking would have handed back the double-delivery window this package's interface exists to close. Publish fails CLOSED when INCR fails, where the template falls back to a local counter. Two instances falling back at once mint ids from independent counters into a shared stream, and replayBuffer.since() reasons on monotonicity — so the damage is silent replay corruption, not a visible error. INCR and PUBLISH share a connection anyway, so the fallback mostly lets a doomed publish proceed carrying a poisoned id. Both load-bearing tests were VACUOUS as first written; the mutation matrix is the only reason I know: - the concurrency test's producer finished before the subscriber joined, so the channel leg was never exercised and a split-lock mutant survived 50 iterations. Now paced, with a both-legs-non-empty precondition that fails a run which never approached the boundary, plus a dedicated detector (600 attempts, 8/8 kills, 0.02s after switching the drain to non-blocking — exact, because the duplicate is already buffered when the call returns). - the fail-closed test asserted nothing was delivered, which is true of the fallback too: Publish never delivers locally, so with Redis down neither policy delivers. Rewritten around a go-redis ProcessHook that records attempted commands, which is where the policies actually differ (INCR-then-stop vs INCR-then-PUBLISH). Also corrects session_presence.go, which told the next person these two had to be fixed together. Delivery is now cross-instance; the registry's under-report is unchanged, so the remaining defect is a picker that under-reports rather than a push that lies. The PLAN-2558 S3 gate stays, for that reason instead of the old one. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * fix(watchevents): make id assignment and publish atomic; close the bus on shutdown (Codex round 1) P1 — INCR and PUBLISH as two client calls are not order-preserving, and the failure is concrete: A gets id 1 and is descheduled, B gets id 2 and publishes, A publishes 1. Every subscriber receives 2 before 1, the replay buffer appends in ARRIVAL order, and replayBuffer.since() reasons on monotonicity — so a resume from 2 hits the sinceID > newestID branch and answers 'gap too large', turning a healthy reconnect into a spurious sync_required, while a resume from 1 silently skips the late arrival. Fixed at the source with a Lua script: Redis runs it atomically on its single thread, so INCR and PUBLISH for one instance both complete before another's script begins, and publish order equals id order globally with no coordination on our side. The id rides as a '<id>|<json>' prefix rather than being edited into the JSON from Lua; the id is digits and the FIRST '|' separates, so a '|' in the body is unambiguous. A pleasant consequence: there is no longer a window where an id exists but the publish has not happened, so the fail-closed decision and the publish decision became the same decision. P2 — Stop() never closed the watch bus. That was survivable for MemoryBus, whose Close only drops channels; RedisBus holds a receive goroutine and a Redis subscription from construction, so it leaked both for the process's life. Closed after bg.Wait(), so a background producer cannot publish into a bus already tearing down. nits, all real, all in artifacts someone reads: - 'exactly-once delivery' was simply wrong. Redis pub/sub is at-most-once and the local send is deliberately non-blocking. The property the round trip actually buys is NO DOUBLE DELIVERY to the publishing instance; the comment now says that and names the replay buffer as the bounded recovery mechanism for the rest. - the Bus interface comment still said only MemoryBus existed. - cmd_server.go's session-presence note still claimed the same caveat as 'the watch bus directly above', which had just stopped applying. - session_presence.go now says delivery is fixed WHEN PAD_REDIS_URL is set, rather than unconditionally. Tests: the fail-closed assertion moved from 'nothing was delivered' — still true under the two-call version — to 'no bare INCR or PUBLISH was issued', which is what distinguishes atomic from not. Mutation-verified by splitting the script back into two calls. Added a decode round-trip test covering the new wire format, a '|' inside the body, and four malformed payloads, since that decoder consumes bytes from a channel any holder of the Redis credentials can publish to. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * fix(watchevents,server): correct the targeted-push claim; close the bus before HTTP shutdown (Codex round 2) P2 — I claimed cross-instance DELIVERY was fixed. Half true, and the false half was mine to catch: handlers_push.go gates a session-targeted push on the LOCAL presence registry and skips the publish entirely when the id is not there, so a POST landing on A for a session held on B still delivers nothing. The bus would carry it; the gate means it never reaches the bus. Broadcast pushes and every other notification kind ARE fixed. I asserted that behaviour from reading the bus and session_presence.go without reading the push handler — the exact thing I hold myself to not doing. Corrected in all three places the claim was made (the package doc, session_presence.go, and the KindPush comment), with the correction recorded rather than quietly overwritten. The gate's own justification is now stale too, and worth more than a tweak: 'a target this instance cannot see is a guaranteed no-op' was TRUE under MemoryBus and is FALSE under RedisBus, where another instance may hold that session. Left in place deliberately — publishing unconditionally would fix delivery and immediately make delivered_sessions=0 a lie in the other direction, which is a question about what that field promises. It belongs with the shared-state SessionPresence that PLAN-2558 S3 already gates on: fixing the registry makes the snapshot right, and then the skip is correct again for its original reason. Both open halves collapse into that one implementation. P2 — the watch bus was closed only in Server.Stop(), which runs AFTER http.Server.Shutdown. The event bus is closed before Shutdown precisely so its SSE handlers unblock; the watch stream is the same shape, so an open one would have held Shutdown to its full 30s deadline. Now closed alongside eventBus, with the Stop() close kept as the path for other callers — both implementations are idempotent. nit — MemoryBus and RedisBus disagreed after Close: RedisBus handed a late Subscribe an already-closed channel, MemoryBus registered one nobody would ever close, so a consumer racing shutdown blocked forever. MemoryBus now matches, and its Close is idempotent, which the CLI's double close relies on. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * fix(watchevents): report a missed notification as a replay gap (Codex round 3) P2 — a divergence MemoryBus structurally cannot have. It assigns every id itself, so its replay buffer is contiguous and the only gap it can report is eviction. RedisBus receives ids over at-most-once pub/sub, so a blipped subscription can miss 101 and receive 102: the buffer holds a hole, is nowhere near full, and replayBuffer.since() answers a resume from 100 with just [102]. The consumer loses a nudge and is never told. RedisBus now tracks the id at which the sequence resumed after the most recent hole, and answers nil — the same signal eviction already gives, which the SSE handler already turns into sync_required — for a resume that would have to span it. Resumes that do not span it still replay normally, and sinceID=0 is treated as a fresh subscriber rather than a resume, so a hole nobody spanned is not turned into a spurious resync. The atomic publish script is what makes this readable: publish order is id order globally, so a non-consecutive id means MISSED, not reordered. Mutation-verified by disabling the check; the test fails on both the spanning resumes and would have failed the over-broad version too (it asserts the non-spanning resumes still work). Two residuals documented rather than fixed, both because the fix is the same shared-state SessionPresence that PLAN-2558 S3 gates on: - delivered_sessions is now wrong in BOTH directions for a broadcast push — the count is local while delivery is global, so a replica can report 1 while two sessions receive it, or 0 while a remote one does. No local arithmetic fixes that; it is asking one replica what all of them are doing. - the Redis channel and counter names are not deployment-scoped, so two installations sharing a Redis endpoint cross-feed (and picking different logical DBs does not help — pub/sub ignores them). Left flat to match internal/events rather than giving one of the two buses a prefix the other lacks; the rule is one Redis endpoint per installation, and relaxing it should cover both buses at once. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * fix(watchevents): a cold-started replica must report a gap too (Codex round 4) P1 — the round-3 hole check only fired BETWEEN two received messages, so it never fired for the first one. A replica restarting while Redis is already at 101 has an empty buffer; its first received message is 102, nothing looks like a hole, and a client reconnecting to that replica with Last-Event-ID 100 was handed [102] — skipping 101 exactly as silently as the case round 3 fixed, by a different route. Replaced contiguousFrom with knownFrom: the lowest id from which this instance's buffer is contiguous. SET on the first append (before which this instance knows nothing) and RESET on every hole (before which it no longer knows anything usable). One variable, both failures. The boundary is pinned in both directions, which is what stops this being an over-broad 'always gap after a restart': a resume from exactly the id before our first (101 when we started at 102) IS contiguous with our view and replays normally. Mutation-verified by disabling the cold-start arm. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * fix(watchevents): idempotent publish, confirmed subscription, and real Redis tests (Codex round 5) P2 — go-redis retries a command whose reply is lost to a network error, and the publish script was not idempotent: the same notification would be published twice under two different ids. Both copies look valid — ordered, distinct — so nothing downstream could tell them apart, and on the push path a duplicate is a duplicate DISPATCH into an agent harness. The script now takes a caller-generated token and SET NX's it, so a retry carrying the same arguments returns 0 without publishing. TWO THINGS THIS UNIT OWES ITS TESTS, both found within minutes of each other and both invisible to the hermetic ones: 1. The idempotency script shipped indexing ARGV[3] while Publish passed two arguments. Caught by re-reading, which is not a control worth relying on for the next Lua edit. 2. NewRedisBus returned before go-redis had established the subscription, so notifications published in that window were lost to this instance, silently. Surfaced as a test flake; the production shape is a rolling deploy, where a replica takes traffic before its subscription is live. The constructor now waits for the confirmation (bounded, and a failure is logged rather than fatal since Channel() re-subscribes on reconnect). So miniredis is now a test dependency, and the round-trip tests it enables cover what fanOutLocally-driven tests structurally cannot: the channel name, the KEYS/ARGV mapping, the id prefix wire format, the shared counter across two buses, cross-instance delivery (the actual bug), the dedupe token, and Close tearing down the SERVER-side subscription rather than just local channels. Verified by restoring the ARGV[3] bug: the round-trip test fails on it. The two findings I am NOT fixing here are unchanged and documented where the reasoning is met — the targeted-push gate and delivered_sessions are both consequences of the per-process presence registry, and both are closed by the shared-state SessionPresence that PLAN-2558 S3 gates on, not by anything in this package. make vuln: 0 vulnerabilities in imported packages. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * fix(watchevents): survive a Redis counter reset without replaying stale ids (Codex round 6) P2 — pad:watchevents_seq has no TTL but can still vanish: evicted under maxmemory, dropped by a FLUSHDB, or restored from an older snapshot. Ids then restart at 1 while this instance's ring still holds the hundreds. Keeping both is what corrupts replay — the two id spaces are not comparable, so a resume from 2 in the NEW space would be handed the stale 99/100/101 as though they were newer. A backwards id now drops the replay buffer and re-anchors knownFrom. Every resume from the old space then exceeds the newest id held and gets nil — the resync signal that is the only honest answer once the ids stopped meaning what the client thinks they mean — while clients in the new space keep working immediately. The test asserts BOTH halves, which is what makes it a detector rather than a description: a build that logged the reset and kept the buffer passes 'the old resume reports a gap' and fails 'the new resume never returns a pre-reset entry'. Mutation-verified on exactly that. Hardened while I was here: the epoch-reset path REBUILDS the buffer at runtime, so a bus constructed with a non-positive replay size would have turned a counter reset into a panic (newReplayBuffer(0)'s first append indexes a zero-length slice) rather than a resync. The constructor now normalizes. MemoryBus has the same trap for a caller passing 0; left alone as pre-existing and off this path, but named in the comment rather than silently fixed or silently ignored. nit — this file's header still claimed there was no miniredis dependency and no round-trip coverage, which the previous commit made false. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * docs(watchevents): actually correct the hermetic test header (Codex round 7) The previous commit's message claimed this fix. It did not contain it: the edit ran as one of two scripts in a single command, its assertion failed with a traceback, and the second script's success is what I read. The header kept saying there was no miniredis dependency and no round-trip coverage — both false since two commits ago, in the file a reader consults to find out what IS covered. That is the adjacent-success-signal failure exactly: a success line from the step next to the one I cared about. The tell was in the output and I walked past it, then asserted the change in a commit message. Recording it here rather than quietly fixing, because a commit that claims a change it does not make is worse than one that omits it. Verified this time by reading the file back and grepping for the stale phrases: zero. Round 7's other three findings are the documented residuals re-raised for the third time — the targeted-push gate, delivered_sessions, and the unnamespaced Redis keys. All three are dispositioned at the line a reader meets them, all three are consequences of the per-process SessionPresence registry or of matching internal/events' existing convention, and none is fixable inside this package. They stay open, on the record, and with the lead. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * docs(watchevents,cli): correct pad push --help; document the reset-window residual (Codex round 8) nit, and the one that stings — cmd_push.go's Long help still said pushes go over the 'in-memory watch-events bus'. That is the text a user reads when they run pad push --help, and it has been false since this branch's first commit. I have a standing pre-push step to grep the artifacts a CONSUMER reads for exactly this, and I ran it as a code search (watchevents.New) rather than a prose search, so --help never came up. The help now distinguishes broadcast (reaches every instance) from session-targeted (still resolved against the handling server) and names the bug. P2 — the counter-reset handling fires when the first post-reset notification ARRIVES, so there is a window between Redis losing the counter and the next publish in which this instance still replays old ids to a reconnecting client. Documented as accepted rather than closed: nothing local can detect the reset earlier (the counter is in Redis and we learn of it by receiving something), and the two shapes that would — a GET per resume, or a background poller — put network I/O on a latency-sensitive path or spend a goroutine and a round trip per tick forever against a condition measured in years. The exposure is redelivery of notifications the client already has, bounded by the window and self-healing on the next publish. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * fix(watchevents): a replica that has received nothing must not answer 'caught up' (Codex round 9) P1 — the coverage check was skipped entirely while knownFrom was still 0, so a bus that had received NOTHING answered any cursor with an empty-but-non-nil replay, which the SSE handler reads as caught-up. The scenario is a restart, not an exotic one: replica B comes up while Redis is at 100, id 101 is published before B's subscription is live, and a client reconnects to B with Last-Event-ID 100 before 102 arrives. B says caught-up, then delivers 102 live, and 101 is gone with nothing to tell anyone. The principle the code now follows: having received nothing is strictly LESS knowledge than 'contiguous from X', so it must produce at least as strong a signal. A non-zero cursor against an empty bus is a gap. Both sides pinned, because the over-broad version is a real risk here — answering every fresh connection with a resync would be its own bug. A sinceID of 0 is not a resume and still gets an empty replay rather than a gap. Mutation-verified on the new arm. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * docs(watchevents,cli): name the trailing-gap and shutdown trades (Codex round 10) Two findings that are decisions rather than defects, so both are documented at the line where the reasoning is met and taken to the plan instead of being settled unilaterally after ten review rounds. P1 as reported — the TRAILING gap. Everything the coverage bookkeeping does reasons about what this instance HAS received; it cannot see a notification missed at the END of the sequence. Hold 100, miss 101 to a disconnect, and a client resuming from 100 before 102 arrives is told caught-up. The hole only becomes visible when 102 lands, which is too late for that connection. What would reveal it is a GET of the sequence key: a value above lastAppendedID means ids exist we never saw, and a value BELOW it reveals the counter reset documented last round — one mechanism, both open windows. It is not done here because it is product-visible in the other direction: INCR happens before the message propagates, so the counter legitimately runs ahead of every instance for microseconds after each publish, and a strict comparison turns ordinary in-flight traffic into spurious sync_required responses with no principled tolerance to pick. A resync is recoverable and a lost nudge is not, which is the argument for doing it — but that is a call about how chatty the resync path should be. P2 — closing the watch bus before Shutdown drains handlers means a push already in flight can publish into a closed bus and still return 200 with pushed:true. Closing after would instead hold every shutdown to its 30s deadline on any open stream. eventBus already makes the same trade the same way; naming it rather than inheriting it silently. The honest fix is Bus.Publish reporting the drop so the handler can, which is an interface change and a different unit. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * feat(watchevents): close the trailing gap with a settle-window authority check (lead ruling) Lead's ruling on BUG-2651: a silently lost nudge is unbounded staleness, a spurious resync costs one redundant fetch, so the gap must not survive — and don't pick a magnitude tolerance, because the reason the counter legitimately runs ahead is in-flight propagation, which is TIME-bounded while a genuinely missed message never arrives. So the discriminator is time. On a resume (and only on a resume), read the shared counter: if it disagrees with this instance's high-water mark, wait out one settle window and read again. In-flight ids land during the beat and the resume proceeds normally; missed ones never do and the resume is answered with a gap. That converts an unprincipled 'how many ids behind is too many' threshold into a principled propagation bound. The same read also catches the counter having gone BACKWARDS, so the counter-reset window documented last round is closed by the same mechanism rather than needing its own — the arrival-time reset handling stays, because it is what repairs the instance's own state and what covers a bus with no reconnecting clients. Ordering matters and is documented at the call: the check runs WITHOUT the mutex (it sleeps and does network I/O, neither of which may happen inside the lock fan-out needs) and BEFORE subscribing rather than between subscribe and replay, which would reopen the double-delivery window SubscribeAndReplaySince exists to close. Nothing is lost by waiting first — fanOutLocally buffers regardless of subscribers. An unreadable counter falls back to local knowledge rather than failing closed: turning a Redis hiccup into a resync for every reconnecting client at once is a worse failure than the one being guarded against. EventsSince deliberately does NOT do this and says so — it is the local primitive the Bus interface already describes as being for tests and non-resuming callers, and making it sleep and hit the network would surprise every one of them. Five tests, each pinning a different half: the missed tail reports a gap; a current instance does NOT (the control that stops this being 'always resync'); an id arriving mid-settle is tolerated; an unreadable counter falls back; a fresh subscriber neither waits nor gets a gap. Mutation-verified twice — disabling the check, and removing the settle beat — each killed by the test that names it. Also filed at the lead's direction, so the two remaining cross-instance defects have tracked homes rather than only comments: BUG-2698 (targeted push resolved against local presence, plus the delivered_sessions inaccuracy — one shared-state SessionPresence closes both) and BUG-2699 (push returns 200 pushed:true for a dropped publish; Bus.Publish reports nothing, and fixing it is an interface change). Every disposition comment now cites its item. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * fix(watchevents): compare two FRESH reads, not one stale snapshot (Codex round 11) P1 — the settle beat re-read only the local side, so the comparison was against a counter SNAPSHOT taken before the wait. Id 2 arrives during the beat while id 3 is published and missed: the stale remote is still 2, the check declares convergence, and 3 is silently lost — the exact failure this whole mechanism exists to prevent, reintroduced inside it. P2 — the same staleness in the other direction. A GET can land just before a publish completes and report a value BELOW what this instance already holds; that never matches, so a client who had missed nothing got a full resync. Both are one defect: agreement between the authority and this instance has to be evaluated on two FRESH reads or it is not agreement. Now re-reads both sides after the beat, and treats any remaining disagreement as a gap in either direction — still behind means ids never reached us, still ahead means the counter was reset under us and our buffer belongs to a dead id space. Two tests, one per direction, each mutation-verified against the re-read-locally-only version: the second counter advance must produce a gap, and the raced read must NOT produce a resync. Without the second test the fix could have been 'always report a gap', which passes the first. Documented the cost side of the lead's ruling while I was in here: the condition is agreement, so a resume during CONTINUOUS publishing across the whole settle window can disagree every time and resync. Bounded by this stream being low-volume by design and resumes only happening on reconnect; if a workload makes it chatty, the answer is a longer window, not a magnitude threshold. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * fix(watchevents): an absent sequence key is zero, not unreadable (Codex round 12) P2 — the counter key can DISAPPEAR after this bus has seen ids (FLUSHDB, eviction). Reading redis.Nil as 'unreadable' meant falling back to local knowledge and cheerfully replaying an id space the authority no longer has — while the next publish starts again at 1 and collides with it. Absent is a VALUE. Returning zero-and-readable makes the case fall out of the ordinary comparison with no special branch: an instance holding 101 disagrees with an authority at 0, does not converge, and the resume is answered with a gap. A genuinely fresh deployment still agrees at zero and is not resynced — which is the control leg, and the reason 'absent means gap' would have been the wrong fix: it passes the first test while resyncing every first connection on a new install. P1 as reported — the equality fast path returning without settling — is not closed, and the comment now says why rather than leaving it to be re-found. A notification published AFTER that read and missed by this instance is invisible to any check made here, and settling anyway would not close it: the same race exists in the instant after the function returns. The check's honest scope is what was missed BEFORE the resume. A message missed after it is a property of at-most-once pub/sub with no per-connection ack, and the real answer is a durable stream (Redis Streams with consumer groups), not a longer wait. Mutation-verified: restoring redis.Nil to the unreadable branch fails the disappearing-counter test. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * feat(watchevents): epoch marker, so a reset that caught up is still a reset (Codex round 13) P2 — numeric detection is blind to a reset that has already climbed past this instance's high-water mark. Hold 100, lose the connection, the counter resets and ids 1-101 are published, and the only one that reaches us is 101 — the perfect contiguous successor of 100. Every arithmetic check passes, the buffer quietly mixes two id spaces, and a client resuming from OLD 100 is handed NEW 101 having silently missed the new space's 1-100. No amount of comparing numbers fixes that, because the question is not 'is this bigger' but 'is this the same sequence'. The publish script now mints an epoch once per id space (SET NX, so every publisher can offer one and the first wins) and carries it on every message; a change drops the buffer and re-anchors. The subtle half, and the one the first attempt got wrong: after an epoch change the cold-start rule must NOT admit its usual contiguous-with-our-view cursor. Within an epoch, a client at n.ID-1 is genuinely adjacent to our first id. Across one it is ambiguous — id spaces overlap, so that cursor may be the OLD sequence's n.ID-1, a different notification entirely — and admitting it hands them the new epoch's id as though it followed theirs, which is exactly the failure the epoch exists to prevent. Letting it back in one line later would have been a poor joke. The test caught it; the control leg (a cursor genuinely inside the new epoch is still served) is what stops the fix becoming 'resync everyone forever after any reset'. Wire format changed to <epoch>|<id>|<json>. Free of compat cost, checked rather than assumed: redis_bus.go does not exist on origin/main, so no released build produces or consumes the old shape. The numeric backward check stays — it covers a counter reset where the epoch key survived (eviction picks keys individually), and it is what repairs an instance with no reconnecting clients at all. Mutation-verified: ignoring the epoch change fails the new test. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * docs(watchevents): the wire format comments say <epoch>|<id>|<json> (Codex round 14) Three comments still described the pre-epoch format. Worth more than a tidy-up: a maintainer following them would conclude the epoch prefix is vestigial and remove it, which reintroduces exactly the cross-epoch replay corruption round 13 existed to fix. The publishScript comment now also says outright that the epoch is not decoration and points at redisWatchEpochKey before anyone considers it removable. Verified by grepping for the old shape rather than by trusting the edits — zero remaining, which is the check I owed after getting this wrong in round 7. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V * chore(nix): update vendorHash for the miniredis test dependency (BUG-2651) CI's Nix job failed on a fixed-output hash mismatch, and it is neither a flake nor a surprise once seen: nix/package.nix pins the vendored module set, and adding miniredis (plus gopher-lua, its Lua interpreter) to go.mod changed it. Regenerated per the procedure the file itself documents — build and read the 'got:' line. Run on CI rather than locally because this box has no nix; the hash is a content hash of the module set determined by go.mod/go.sum, so the same inputs produce it in either place. Worth naming as a gate lesson rather than just fixing: my pre-merge matrix had build, lint, test, test-pg, vuln and Codex, and none of them can see this. A dependency change has a SEVENTH consumer — the Nix packaging — and the only thing that checks it is the CI job that just did. Adding a dependency means checking the packaging, not only the security scan. Claude-Session: https://claude.ai/code/session_017jD6t1zjxGSq47SQpZfp1V |
||
|
xarmian
|
ad1e919290 |
chore(deps): bump otel exporters to v1.45.0, clearing GO-2026-4985 from the Nix baseline (#1097)
* chore(deps): bump otel exporter cluster to v1.45.0 (GO-2026-4985) Clears GO-2026-4985 (otlptracehttp oversized response bodies, fixed v1.43.0) from the Nix artifact's accepted-advisories baseline. The whole cluster is transitive — pad has no direct otel usage; it arrives via fosite → ory/x → otelx, and fosite's latest (v0.49.0, already pinned) still requires the vulnerable exporter, so MVS override is the only path. Pulls otel core/metric/sdk/trace v1.44→v1.45, proto/otlp v1.0.0→v1.11.0, grpc v1.82.1→v1.83.0, genproto refresh. The jaeger exporter stays at v1.17.0 (its final release) and coexists. BUG-2085 deferred this bump pending a blast-radius assessment; the assessment is this diff, measured: go build ./..., go vet, full SQLite test suite, and golangci-lint all green; artifact-faithful proxy scan (GOTOOLCHAIN=go1.26.5, -s -w) reports 9/9 accepted with no new advisories. Remaining baseline: 8 stdlib (nixos-26.05 backport) + openpgp (no upstream fix exists). vendorHash refresh follows in the next commit via the PR's Nix CI run. Refs BUG-2085, BUG-2567. Claude-Session: https://claude.ai/code/session_01BhQoeaWXxJbvw86ezzK8dt * chore(nix): refresh vendorHash for the otel exporter bump Same flow as #1096: value from the PR's own failed Nix CI run. Claude-Session: https://claude.ai/code/session_01BhQoeaWXxJbvw86ezzK8dt |
||
|
xarmian
|
ff201939b9 |
chore(deps): bump x/image to v0.45.0, clearing GO-2026-6222 from the Nix baseline (#1096)
* chore(deps): bump golang.org/x/image to v0.45.0 (GO-2026-6222) Clears GO-2026-6222 (VP8L decode memory allocation) from the Nix artifact's accepted-advisories baseline — the advisory's fixed version is exactly v0.45.0. Pulls x/text v0.41.0, x/mod v0.38.0, x/tools v0.48.0 as transitive requirements. Verified against a build-faithful proxy (GOTOOLCHAIN=go1.26.5, -s -w): scan reports 10/10 accepted, no new advisories, no prune warnings. Full SQLite test suite and golangci-lint clean locally. nix/package.nix vendorHash refresh follows in the next commit, using the PR's Nix CI job as the builder (no local nix; the flow is the one package.nix documents). Refs BUG-2085, BUG-2567. Claude-Session: https://claude.ai/code/session_01BhQoeaWXxJbvw86ezzK8dt * chore(nix): refresh vendorHash for the x/image bump Codex round 1 P1: go.sum changed, so buildGoModule's fixed-output vendor derivation no longer matches the pinned hash. Value taken from the PR's own failed Nix CI run (the got: line), per the regeneration flow package.nix documents. Claude-Session: https://claude.ai/code/session_01BhQoeaWXxJbvw86ezzK8dt |
||
|
xarmian
|
8af62d1c4e |
fix(ci): build with go1.26.6 to clear the govulncheck gate (BUG-2565) (#1093)
* fix(ci): build with go1.26.6 to clear the govulncheck gate (BUG-2565)
main has been red since
|
||
|
dependabot[bot]
|
ca45539925 |
chore(deps)(deps): bump the go-minor-and-patch group across 1 directory with 13 updates
Bumps the go-minor-and-patch group with 10 updates in the / directory: | Package | From | To | | --- | --- | --- | | [github.com/JohannesKaufmann/html-to-markdown/v2](https://github.com/JohannesKaufmann/html-to-markdown) | `2.5.1` | `2.5.2` | | [github.com/go-chi/chi/v5](https://github.com/go-chi/chi) | `5.2.5` | `5.3.1` | | [github.com/jackc/pgx/v5](https://github.com/jackc/pgx) | `5.9.2` | `5.10.0` | | [github.com/mark3labs/mcp-go](https://github.com/mark3labs/mcp-go) | `0.56.0` | `0.57.0` | | [github.com/pb33f/libopenapi](https://github.com/pb33f/libopenapi) | `0.36.3` | `0.38.7` | | [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) | `1.23.2` | `1.24.1` | | [github.com/redis/go-redis/v9](https://github.com/redis/go-redis) | `9.19.0` | `9.22.0` | | [github.com/santhosh-tekuri/jsonschema/v6](https://github.com/santhosh-tekuri/jsonschema) | `6.0.2` | `6.0.3` | | [golang.org/x/image](https://github.com/golang/image) | `0.43.0` | `0.44.0` | | [modernc.org/sqlite](https://gitlab.com/cznic/sqlite) | `1.50.0` | `1.56.0` | Updates `github.com/JohannesKaufmann/html-to-markdown/v2` from 2.5.1 to 2.5.2 - [Release notes](https://github.com/JohannesKaufmann/html-to-markdown/releases) - [Commits](https://github.com/JohannesKaufmann/html-to-markdown/compare/v2.5.1...v2.5.2) Updates `github.com/go-chi/chi/v5` from 5.2.5 to 5.3.1 - [Release notes](https://github.com/go-chi/chi/releases) - [Changelog](https://github.com/go-chi/chi/blob/master/CHANGELOG.md) - [Commits](https://github.com/go-chi/chi/compare/v5.2.5...v5.3.1) Updates `github.com/jackc/pgx/v5` from 5.9.2 to 5.10.0 - [Changelog](https://github.com/jackc/pgx/blob/master/CHANGELOG.md) - [Commits](https://github.com/jackc/pgx/compare/v5.9.2...v5.10.0) Updates `github.com/mark3labs/mcp-go` from 0.56.0 to 0.57.0 - [Release notes](https://github.com/mark3labs/mcp-go/releases) - [Commits](https://github.com/mark3labs/mcp-go/compare/v0.56.0...v0.57.0) Updates `github.com/pb33f/libopenapi` from 0.36.3 to 0.38.7 - [Release notes](https://github.com/pb33f/libopenapi/releases) - [Commits](https://github.com/pb33f/libopenapi/compare/v0.36.3...v0.38.7) Updates `github.com/prometheus/client_golang` from 1.23.2 to 1.24.1 - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/v1.24.1/CHANGELOG.md) - [Commits](https://github.com/prometheus/client_golang/compare/v1.23.2...v1.24.1) Updates `github.com/redis/go-redis/v9` from 9.19.0 to 9.22.0 - [Release notes](https://github.com/redis/go-redis/releases) - [Changelog](https://github.com/redis/go-redis/blob/master/RELEASE-NOTES.md) - [Commits](https://github.com/redis/go-redis/compare/v9.19.0...v9.22.0) Updates `github.com/santhosh-tekuri/jsonschema/v6` from 6.0.2 to 6.0.3 - [Release notes](https://github.com/santhosh-tekuri/jsonschema/releases) - [Commits](https://github.com/santhosh-tekuri/jsonschema/compare/v6.0.2...v6.0.3) Updates `golang.org/x/crypto` from 0.53.0 to 0.54.0 - [Commits](https://github.com/golang/crypto/compare/v0.53.0...v0.54.0) Updates `golang.org/x/image` from 0.43.0 to 0.44.0 - [Commits](https://github.com/golang/image/compare/v0.43.0...v0.44.0) Updates `golang.org/x/term` from 0.44.0 to 0.45.0 - [Commits](https://github.com/golang/term/compare/v0.44.0...v0.45.0) Updates `golang.org/x/text` from 0.39.0 to 0.40.0 - [Release notes](https://github.com/golang/text/releases) - [Commits](https://github.com/golang/text/compare/v0.39.0...v0.40.0) Updates `modernc.org/sqlite` from 1.50.0 to 1.56.0 - [Changelog](https://gitlab.com/cznic/sqlite/blob/master/CHANGELOG.md) - [Commits](https://gitlab.com/cznic/sqlite/compare/v1.50.0...v1.56.0) --- updated-dependencies: - dependency-name: github.com/JohannesKaufmann/html-to-markdown/v2 dependency-version: 2.5.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-and-patch - dependency-name: github.com/go-chi/chi/v5 dependency-version: 5.3.1 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: github.com/jackc/pgx/v5 dependency-version: 5.10.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: github.com/mark3labs/mcp-go dependency-version: 0.57.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: github.com/pb33f/libopenapi dependency-version: 0.38.7 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: github.com/prometheus/client_golang dependency-version: 1.24.1 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: github.com/redis/go-redis/v9 dependency-version: 9.22.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: github.com/santhosh-tekuri/jsonschema/v6 dependency-version: 6.0.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-and-patch - dependency-name: golang.org/x/crypto dependency-version: 0.54.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: golang.org/x/image dependency-version: 0.44.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: golang.org/x/term dependency-version: 0.45.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: golang.org/x/text dependency-version: 0.40.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: modernc.org/sqlite dependency-version: 1.56.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch ... Signed-off-by: dependabot[bot] <support@github.com> |
||
|
xarmian
|
cdb495d400 |
chore(deps): bump google.golang.org/grpc to v1.82.1 for GO-2026-6061 (BUG-2361) (#1050)
grpc v1.79.3 is affected by GO-2026-6061 (xDS RBAC authorization engine and HTTP/2 transport server). It reaches us indirectly via internal/oauth -> ory/fosite -> ory/x/otelx -> otlptracehttp -> grpc. The bump pulls otel v1.42.0 -> v1.43.0 as grpc's required floor, which re-exposes GO-2026-5158 (uncapped baggage header parsing). Per the otel lockstep policy, otel/metric/trace/sdk all move together to v1.44.0, which clears it. make vuln is now completely clean (0 call-reachable vulnerabilities). Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT |
||
|
xarmian
|
a94f2d37d4 |
fix(deps): bump otel to v1.42.0 — clears GO-2026-5506 + GO-2026-5158, un-reds main CI (#1026)
* fix(deps): bump go.opentelemetry.io/otel family to v1.42.0 (GO-2026-5506, GO-2026-5158) CI's Go job has been red on main since GO-2026-5506 published (reachable baggage/propagation symbols in otel v1.40.0). v1.41.0 fixes it but carries GO-2026-5158 (fixed in v1.42.0), so bump straight to v1.42.0 (otel + metric + trace in lockstep; sdk untouched per go mod tidy). Verified: make vuln (binary-mode govulncheck) exit 0, go build ./..., full go test sweep green (24 ok). * fix(deps): bump otel/sdk to v1.42.0 in lockstep with the API (Codex — otel compat policy pairs SDK with API version) |
||
|
xarmian
|
0f48bcebfa |
fix(ci): bump advisory deps to restore green CI (BUG-2278) (#997)
Two newly-published upstream advisories postdate the last green main run and were failing the Go and Web CI jobs on every PR. Both are DoS-class in parsing/text deps; no product code change. Go job (govulncheck binary mode): - GO-2026-5970: infinite loop on invalid input in golang.org/x/text. Bump golang.org/x/text v0.38.0 -> v0.39.0 via `go get` + `go mod tidy`. go mod tidy pulls the coordinated x/* release train it requires (crypto/term/mod/net/sys/tools). govulncheck -mode binary: 0 called. Web job (npm audit --audit-level=high --omit=dev): - linkify-it <=5.0.1 (high, GHSA-v245-v573-v5vm) + dompurify + markdown-it. `npm audit fix` (lockfile-only). Fixes the 3 advisories (audit now reports 0 vulns). As semver-compatible collateral within existing caret ranges it also refreshed the build toolchain (vite 8.0.11->8.1.5, @sveltejs/kit 2.59.1->2.70.1, rolldown rc.18->1.1.5). Tiptap exact-pins held (check:tiptap-pins green). - Also tighten the existing linkify-it security override floor ^5.0.1 -> ^5.0.2 so it expresses the patched minimum for THIS advisory rather than relying on npm's latest-in-range resolution. Lockfile was already at 5.0.2, so npm ci stays in sync (verified). Gates: go vet, go build, govulncheck -mode binary, go test ./... all green; web npm ci, check:tiptap-pins, audit, build, check, test (464) all green. Independent Codex review: CLEAN. Claude-Session: https://claude.ai/code/session_01EZ6yr6pAUFb1uffan912ra |
||
|
xarmian
|
cf09cf7520 |
chore(deps): bump mcp-go to v0.56.0, advance yaml/v4 to rc.6 (TASK-2060) (#916)
Bump github.com/mark3labs/mcp-go v0.52.0 -> v0.56.0 and go.yaml.in/yaml/v4 v4.0.0-rc.4 -> v4.0.0-rc.6. mcp-go v0.56 turns on DNS-rebinding protection by default in the Streamable HTTP server: a request whose accept socket is loopback but whose Host header is non-loopback is rejected with 403. pad-cloud's mcp.getpad.dev vhost sits behind a reverse proxy that forwards to the process over 127.0.0.1 while preserving the original Host, so the new default would 403 every real MCP request. Restore the pre-v0.56 behaviour with WithDisableLocalhostProtection(true) — the transport only mounts in cloud mode and every request is Bearer/OAuth-authed, so the browser-driven rebinding threat the guard targets doesn't apply. yaml/v4 has no stable v4.0.0 (latest tag is rc.6); advance along the RC line rather than migrate the four artifact/openapi yaml.Node call sites to yaml.v3 (format-sensitive, higher-risk). Zero code churn. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF |
||
|
xarmian
|
d6c6dfe682 |
build(deps): govulncheck binary mode + Go 1.26.5 / x-crypto / gRPC security bumps (#896)
BUG-2084. Two parts. RAM fix: `make vuln` and CI's Go job now run govulncheck in BINARY mode (`-mode binary` against a freshly-built pad binary) instead of source mode (`govulncheck ./...`). Source mode builds an SSA call-graph over the whole dependency tree (BigQuery/OTel/gRPC/Cloud) and balloons to multiple GB of RAM, which was locking up a memory-constrained host. Binary mode reads the binary's symbol table — ~99 MB peak here — while staying call-graph-precise and still detecting stdlib vulns from the Go version stamped in the binary. The scan binary is written to the repo root (real disk, gitignored), never /tmp, since some hosts mount /tmp as a small RAM-backed tmpfs where a large embedded binary can hit "no space left" and consume the RAM we're sparing. Vuln fix (govulncheck binary mode: 0 vulnerabilities after): - go 1.26.4 -> 1.26.5: clears the only CALLED vuln GO-2026-5856 (crypto/tls) plus not-called os GO-2026-4970. - golang.org/x/crypto v0.51.0 -> v0.52.0: clears 13 not-called advisories. - google.golang.org/grpc v1.59.0 -> v1.79.3: clears GO-2026-4762 (gRPC authorization bypass). pad runs no gRPC server, but grpc.Server.Serve ships transitively (OTel/ory/grpc-gateway) so binary mode flags the symbol. Contained 12-line go.mod bump (genproto/protobuf/oauth2 family), no cascade. Remaining not-called advisories deferred to a follow-up dependency sweep: GO-2026-4985 (otel otlptracehttp) and GO-2026-5932 (x/crypto, Fixed in: N/A). |
||
|
xarmian
|
9f85a85427 |
fix(deps): clear CI vuln audits (x/image TIFF CVEs, linkify-it ReDoS) (#772)
CI on main has been red since 2026-06-25 on two dependency-audit gates, not on tests or code: - govulncheck: golang.org/x/image@v0.41.0 carries two TIFF-decoder CVEs (GO-2026-5066 out-of-bounds strip offset panic, GO-2026-5062 unbounded tile sizes) reached via internal/attachments/processor_purego.go -> image.Decode -> tiff.Decode. Both fixed in v0.43.0. The go mod tidy that follows pulls the usual transitive x/{text,mod,sync,tools} bumps. - npm audit (--audit-level=high --omit=dev): linkify-it@5.0.0 has a high-severity quadratic-complexity ReDoS (GHSA-22p9-wv53-3rq4), transitive via tiptap-markdown -> markdown-it -> linkify-it. Pinned to ^5.0.1 via an overrides entry (5.0.1 satisfies markdown-it's ^5.0.0). Verified locally: govulncheck clean, go vet/build/test green, npm audit high gate exits 0, web build + svelte-check green. Claude-Session: https://claude.ai/code/session_01HxBkAMiFBtCRJ2tKSCt3ST |
||
|
xarmian
|
9dc46fddbf |
chore: bump Go toolchain to 1.26.4 (TASK-1739) (#698)
govulncheck (make vuln + the CI gate) is red on main: GO-2026-5039 (net/textproto error escaping) and GO-2026-5037 (crypto/x509 hostname parsing), both reachable from our call graph and both fixed in go1.26.4. CI's setup-go resolves "1.26" to the latest patch, so only go.mod needs the bump. |
||
|
xarmian
|
d2bcbd3b9b |
chore(deps,docs): bump x/image v0.41.0 + sync seed ship-playbook codex guidance (#646)
* chore(deps,docs): bump x/image to v0.41.0 (GO-2026-5031/5032) + sync seed ship-playbook codex guidance - golang.org/x/image v0.39.0 → v0.41.0: clears GO-2026-5031/5032 (reachable via attachment image decode). go mod tidy. govulncheck clean. - templates_startup_ship.go (seeded ship playbook for new workspaces): drop the deprecated `codex exec --full-auto`, add `< /dev/null` + a codex-specific note that open stdin causes the zero-output "wedge" (not prompt length), and the stdin-first rule-out in the wedge/safety notes. Matches the ship-tasks skill + PLAYB-1405. * fix(docs): show review prompt as positional arg in seed ship-playbook example per Codex review (round 1) codex exec reads the prompt from stdin when none is passed as an argument, so the `-o <file> < /dev/null` example without a prompt would review nothing. Show the prompt positional and note the gotcha. |
||
|
xarmian
|
08f76f3486 |
fix(deps): bump go-jose/v3 to v3.0.5 to clear GO-2026-4945 (BUG-1619) (#634)
* fix(deps): bump go-jose/v3 to v3.0.5 to clear GO-2026-4945 (BUG-1619)
GO-2026-4945 — Go JOSE panics in JWE decryption. Reachable via
github.com/ory/fosite v0.49.0 from internal/server/handlers_oauth.go's
handleOAuthAuthorize (govulncheck call chain into jose.ParseSigned /
JSONWebSignature.Verify / etc).
Drop-in dependency bump, no API changes. Post-bump govulncheck reports
0 reachable vulnerabilities.
Discovered during v0.6.0 release pre-flight (PLAYB-1160 step 1) — CI
run 26518095093 failed govulncheck after the most recent main push,
even though nothing about that change touched OAuth. Same shape as
TASK-1583 (the golang.org/x/net bump earlier this cycle).
* test(open-children-guard): filter readParentRef by SourceRef to fix race flake (BUG-1621)
readParentRef was matching the first LinkType=="parent" link in
GET /items/{ref}/links's response — but GetItemLinks returns links
in BOTH directions (WHERE source_id = ? OR target_id = ?). After
TestOpenChildrenGuard_PatchAtomicRejectionPreservesParentLink attaches
a child task to its target plan, the response contains two parent
links: target→oldParent AND child→target. ORDER BY created_at DESC
ties at sub-microsecond resolution; SQLite tiebreaks on rowid; under
race-scheduler CI load the child→target link sometimes lands first,
the helper returns the target's OWN ref as the "parent", and the
setup-sanity assertion fails with "parent should start as PLAN-1,
got PLAN-3" (where PLAN-3 = target itself).
Filter on SourceRef == itemRef so only the item's OUTGOING parent link
matches. Test-file-only change.
Latent since PR #571 (IDEA-1494,
|
||
|
xarmian
|
8cf460b381 |
fix(ci): unbreak Go (PostgreSQL) test + clear x/net govulncheck findings (#618)
* fix(test): encode bools as bools in TestGetUserWorkspacesDetailed (BUG-1582) The raw INSERT in this test passed integer literals `0, 0, 1` for `sort_order, is_default, is_system`. SQLite coerces int → bool but pgx refuses, so the Go (PostgreSQL) CI job has failed every run since #600 landed: workspace_members_admin_detail_test.go:42: seed system collection: failed to encode args[10]: unable to encode 0 into binary format for bool (OID 16): cannot find encode plan Pass `false, true` for the two bool columns so both drivers accept the args. * chore(deps): bump golang.org/x/net to v0.55.0 (TASK-1583) Clears 5 govulncheck findings (GO-2026-5025..5030) reachable via internal/urlimport/generic.go's call to html.Parse. The Go CI job has been failing on every main run since these advisories were published. Vulnerability #1: GO-2026-5030 — XSS via duplicate attributes Vulnerability #2: GO-2026-5029 — character refs in DOCTYPE Vulnerability #3: GO-2026-5028 — DoS parsing arbitrary HTML Vulnerability #4: GO-2026-5027 — HTML elements in foreign content Vulnerability #5: GO-2026-5025 — namespaced elements in foreign content `go mod tidy` pulls along the usual x/* sibling bumps. Local govulncheck after the bump: *No vulnerabilities found.* Full `go test ./...` passes. * fix(store): is_system check uses NOT bool, not = 0 (BUG-1582) GetUserWorkspacesDetailed's collections_count subquery had `c.is_system = 0`. SQLite stores BOOLEAN as INTEGER so the comparison worked there, but Postgres' boolean column rejects the integer literal: ERROR: operator does not exist: boolean = integer STATEMENT: SELECT ... AND c.is_system = 0) The first push at this BUG only fixed the test-side encoder issue; this commit fixes the production query that the test exercises. `NOT c.is_system` evaluates correctly on both drivers without needing to thread another placeholder through the args. Verified locally against a real Postgres 17 instance: TestGetUser- WorkspacesDetailed and the full ./internal/store/... suite pass. |
||
|
xarmian
|
8771f95ab2 |
feat(urlimport): OpenAPI 3.x → Markdown converter (TASK-1471) (#555)
* feat(urlimport): OpenAPI 3.x → Markdown converter (TASK-1471)
Adds ConvertOpenAPI to internal/urlimport — the "openapi" branch of
the v1 importer. Built on pb33f/libopenapi.
Layout:
- H1 with the API title + version + description
- Contact + License lines
- Servers list
- Endpoints section grouped by primary tag (or "Other" for the
untagged). Per operation: `METHOD /path` heading, summary,
description, deprecation marker, operation ID, parameter table,
request-body summary (with media-type fences and YAML-rendered
example), and response code table.
- Schemas section with component schemas as Property/Type/Required/
Description tables, schema names sorted for stable output.
Scope:
- OpenAPI 3.x only. Swagger 2.0 detection returns an explicit
"only 3.x" error so the import endpoint (TASK-1472) can fall
through to the generic converter.
- Recoverable libopenapi build errors (unresolved refs, etc.) are
swallowed when the model is still produced — partial spec >
no output.
Tests:
- testdata/petstore-openapi.yaml — full v3 fixture: tags, params,
requestBody example, deprecated op, ref-typed schema array, two
component schemas with required-field markers.
- TestConvertOpenAPI_Petstore — 30+ markdown-substring assertions
on the rendered output.
- TestConvertOpenAPI_RejectsSwagger2 — explicit v2 error.
- TestConvertOpenAPI_RejectsGarbage — non-spec input.
- TestConvertOpenAPI_MinimalSpec — empty paths short-circuits.
- Helpers: schemaTypeBrief(nil), escapeTableCell, singleLine.
Dependency: github.com/pb33f/libopenapi v0.36.3 (MIT-licensed).
Parent: PLAN-1467.
* fix(urlimport): merge path-level + operation-level parameters per Codex review (round 1)
MEDIUM: OpenAPI path-item-level parameters apply to every operation
on the path. Previously only slot.op.Parameters was rendered, so
common specs that hoist a shared {id} parameter to the path-item
level emitted operations with the path parameter missing from the
docs.
Now opSlot carries item.Parameters as pathParams, and a new
mergeParameters helper produces the spec-conformant union:
- Path-level parameters first, in declared order.
- Operation-level parameters with matching (name, in) override the
path-level entry in place.
- Operation-only parameters appended after.
Tests:
- TestConvertOpenAPI_PathLevelParametersMerged — inline fixture
with a path-level widgetId + trace and an operation-level trace
override + fields op-only param. Asserts widgetId survives, trace
shows op-level (required=yes), no duplicate path-level trace row,
fields appears.
- TestMergeParameters_EmptyInputs — nil/nil short-circuit.
* fix(urlimport): no double-backticks on array-of-ref schema types per Codex review (round 2)
MEDIUM: schemaTypeBrief() previously wrapped refs in inline backticks
("`Pet`"). For array-of-ref schemas the brief became "array of `Pet`",
and the table-cell call site (codeOrBlank) then wrapped the entire
value in another pair, producing broken markdown like
"`array of `Pet``". Schema properties whose type is an array of a
component schema are a normal OpenAPI shape — `Litter.pets: array of
Pet` — so this would have hit real specs immediately.
Fixes:
- schemaTypeBrief now returns plain text — ref names without
surrounding backticks. Docstring updated to make the contract
explicit ("never contains backticks; caller wraps").
- codeOrBlank strips any stray backticks from input before wrapping
so the resulting cell always carries exactly one balanced pair.
Defensive: the contract from schemaTypeBrief is plain text now,
but stray backticks from any future caller can't corrupt the
table.
Tests added:
- TestConvertOpenAPI_ArrayOfRefTypeCell — inline spec with a
`Litter.pets: array of Pet` property. Asserts the type cell is
exactly `` `array of Pet` `` and no malformed variants leak.
- TestCodeOrBlank — 7-case table covering empty, plain, whitespace,
pre-backticked, embedded-backtick, and backtick-only inputs.
|
||
|
xarmian
|
d1560606cb |
feat(urlimport): generic HTML→Markdown converter (TASK-1470) (#553)
* feat(urlimport): generic HTML→Markdown converter (TASK-1470)
Adds ConvertGeneric to internal/urlimport — the v1 catch-all converter
for "non-OpenAPI" URLs. Pipeline:
1. go-shiori/go-readability strips chrome/nav/ads/scripts and returns
the page's primary article.
2. JohannesKaufmann/html-to-markdown/v2 converts the cleaned HTML to
markdown.
3. cleanupMarkdown normalizes line endings, trims trailing whitespace,
collapses blank-line runs, and ensures a single trailing newline.
Fallback path: when Readability cannot identify an article (directory
listings, single paragraphs, pages with no clear content container),
the converter falls back to a whole-body conversion so callers still
get usable markdown.
Dependencies (license-checked):
- github.com/JohannesKaufmann/html-to-markdown/v2 v2.5.1 (MIT)
- github.com/go-shiori/go-readability (Apache-2.0)
Fixtures + tests:
- testdata/availity-shape.html — Availity-style div soup with heavy
chrome (nav, ads, sidebar, footer, analytics script). Asserts the
article content survives and the chrome is stripped.
- testdata/mdn-shape.html — MDN-style semantic HTML (article/main +
proper heading levels + code fences). Asserts structure preserved.
- TestConvertGeneric_EmptyBody — empty-input rejection.
- TestConvertGeneric_PlainTextFallback — Readability-can't-find-article
fallback path.
- TestCleanupMarkdown — 5-case table-driven cleanup verification.
Parent: PLAN-1467.
* fix(urlimport): preserve hard-line-break markers + apply WithDomain on fallback per Codex review (round 1)
- MEDIUM: cleanupMarkdown was stripping the markdown two-trailing-
spaces hard-line-break idiom. html-to-markdown emits <br> as
" \n" — bulk-stripping trailing whitespace was demoting hard
breaks to soft wraps. Now the cleanup steps line-by-line, keeps
exactly-two trailing spaces (no tab), strips 1/3+/tab-mixed runs.
- MEDIUM: The raw-HTML fallback path (used when Readability cannot
identify an article) now passes converter.WithDomain(pageURL) so
relative links/images resolve against the source URL rather than
the Pad host where they'd 404.
Tests added:
- cleanupMarkdown: hard-line-break preserved, single/triple trailing
spaces stripped, tab-mixed spaces stripped, blank-line-with-spaces
collapsed (5 new cases).
- TestConvertGeneric_RelativeURLsResolvedOnFallback: relative href
in non-article HTML resolves to absolute URL via pageURL.
|
||
|
xarmian
|
2945ee27dd |
feat(server): add WebSocket handler at /api/v1/collab/{itemID} (TASK-1254) (#452)
* feat(server): add WebSocket handler at /api/v1/collab/{itemID} (TASK-1254)
WebSocket entry point for Yjs-based collaborative editing on a
single item under PLAN-1248. Bare-bones in this PR by design:
upgrade + log connect/disconnect + drain reads. Protocol logic
(forwarding to OpBus, persisting to op-log, awareness fan-out)
arrives in TASK-1255 (room manager).
Authorisation mirrors RequireWorkspaceAccess but keyed on the
item's workspace ID rather than a {slug} URL param — the WS URL
only carries itemID. Implementation re-uses the same access
ladder:
fresh-install escape hatch (no users)
→ grant
legacy workspace-scoped API token, no user
→ grant if token's workspace matches the item's workspace
OAuth token allow-list (TASK-953)
→ reject when workspace not on consented list
authenticated user
→ admin OR member OR has guest grants
User is re-fetched from the store on each upgrade (not trusted
from session-context cache) so a mid-session admin demotion or
member removal closes the upgrade path immediately. Mirrors
sseSubscriberStillHasAccess. Periodic per-connection
revalidation lives in TASK-1256.
Route registered alongside SSE (outside the jsonContentType
middleware group, but inside the auth middleware chain). Promotes
github.com/gorilla/websocket from indirect to direct dep and
bumps to v1.5.3 (latest stable; v1.5.0 was already in
go.mod transitively via another package).
Tests cover:
- fresh-install escape hatch grants the upgrade
- bootstrapped server rejects unauthenticated upgrade with 401
- non-member with valid session is rejected with 403
(NOT 401 — confirms the access path runs after auth, not before)
- unknown item surfaces as 404 (not 401/403 leak)
- empty itemID segment doesn't match the route
Test infrastructure note: dialCollab takes an explicit User-Agent
because pad's session-binding middleware hashes the UA at
CreateSession time and re-checks on every request — the dialer
must match what was stored, otherwise the cookie is rejected
before the workspace check fires (and we'd see a misleading 401
where 403 was expected).
Parent: PLAN-1248. Phase 1 — Backend foundation.
* style: gofmt handlers_collab_test.go per Codex review (round 1)
* fix(server): SetReadLimit + nginx upgrade headers for collab WS per Codex review (round 2)
P-MEDIUM #1: handleCollab.ReadMessage had no per-message size cap, so an
authenticated client could send an arbitrarily large frame and force
unbounded server-side buffering — the HTTP body limit applied by the
auth chain doesn't apply once the connection is upgraded. Set
SetReadLimit(1 MiB), generous for keystroke-rate Yjs ops and large
enough for a typical initial-sync state. ReadMessage returns an error
when exceeded, which the existing read loop handles as a normal close.
P-MEDIUM #2: deploy/nginx.conf routed /api/v1/collab/ through the
default `location /` block, which sets `Connection ""` (cleared so HTTP
keepalive works) — that strips the Upgrade header, so WebSocket
upgrades silently fail behind the documented nginx deployment. Add a
dedicated location block with proxy_set_header Upgrade $http_upgrade /
Connection "upgrade", same 24h read/send timeouts as SSE so an idle
editor tab does not get cut off mid-session.
* fix(server): enforce per-item visibility in collab WS upgrade per Codex review (round 3)
P2: authorizeCollabAccess granted upgrade to any workspace member or
guest-with-grants without checking whether THIS specific item was
visible to that user. A restricted member (collection_access=specific)
or a guest with grants on item A could upgrade /api/v1/collab/{itemID}
for an item B in a different collection — they'd see live edits to a
document they have no right to read.
Restructure the access ladder:
1. Workspace-level gate stays as-is: "any access at all?" If no
membership AND no grants → 403 (unchanged).
2. Item-level visibility check added on top, mirroring requireItemVisible
without depending on middleware-set request context (the WS path
doesn't go through RequireWorkspaceAccess):
- VisibleCollectionIDs nil → "all" access → grant.
- Item's collection in the visible set → grant.
- Item-level grant on this exact item → grant (covers guests
given access to a single item rather than a whole collection).
- Else → 404, mirroring requireItemVisible's "don't leak
existence" pattern.
Admin path returns nil before this check, so no change there.
Legacy workspace-scoped API tokens grant editor-equivalent access
on workspace match (predates the grants design); that branch is
untouched since legacy tokens don't have a user identity to scope
per-item grants against.
Test added: TestCollabUpgradeRejectsRestrictedMemberForeignCollection
— member with specific access to collA tries to upgrade for an item
in collB → 404. Existing 5 tests still pass.
* fix(server): strict per-item visibility check + sibling-grant test per Codex review (round 4)
P1 (round 4): VisibleCollectionIDs is broader than full-collection
access — it includes collections "anchored" by an item-level grant
(so the nav can still surface the parent collection of a granted
item). Round 3's check treated every visible collection as full
access; a guest with grant `item:A` could upgrade
/api/v1/collab/{B} for a sibling B in the same collection.
Tighten by mirroring guestResourceFilter / requireItemVisible:
1. Coarse stage stays — collection must be in the visible set.
2. NEW strict stage when the user has item-level grants:
(a) full collection grant on this collection → grant
(b) member's "specific" access list including this collection
→ grant
(c) item grant on THIS exact item → grant
Else → 404 (the visible-set hit was anchored by a sibling's
grant, not by full collection access).
When the user has NO item grants, the coarse-only check is
sufficient — visibility came from full collection access (member's
"specific" list, full collection grant, or "all" access).
Test added: TestCollabUpgradeRejectsGuestWithSiblingItemGrantOnly
— guest with item:A grant tries to upgrade for sibling B in the
same collection → 404 (the bug being regression-tested) AND verifies
the granted item A still upgrades cleanly to 101 Switching Protocols.
|
||
|
dependabot[bot]
|
77dffac945 |
chore(deps)(deps): bump the go-minor-and-patch group across 1 directory with 5 updates (#439)
Bumps the go-minor-and-patch group with 5 updates in the / directory: | Package | From | To | | --- | --- | --- | | [github.com/jackc/pgx/v5](https://github.com/jackc/pgx) | `5.9.1` | `5.9.2` | | [github.com/mark3labs/mcp-go](https://github.com/mark3labs/mcp-go) | `0.50.0` | `0.52.0` | | [github.com/redis/go-redis/v9](https://github.com/redis/go-redis) | `9.18.0` | `9.19.0` | | [github.com/spf13/pflag](https://github.com/spf13/pflag) | `1.0.9` | `1.0.10` | | [modernc.org/sqlite](https://gitlab.com/cznic/sqlite) | `1.47.0` | `1.50.0` | Updates `github.com/jackc/pgx/v5` from 5.9.1 to 5.9.2 - [Changelog](https://github.com/jackc/pgx/blob/master/CHANGELOG.md) - [Commits](https://github.com/jackc/pgx/compare/v5.9.1...v5.9.2) Updates `github.com/mark3labs/mcp-go` from 0.50.0 to 0.52.0 - [Release notes](https://github.com/mark3labs/mcp-go/releases) - [Commits](https://github.com/mark3labs/mcp-go/compare/v0.50.0...v0.52.0) Updates `github.com/redis/go-redis/v9` from 9.18.0 to 9.19.0 - [Release notes](https://github.com/redis/go-redis/releases) - [Changelog](https://github.com/redis/go-redis/blob/master/RELEASE-NOTES.md) - [Commits](https://github.com/redis/go-redis/compare/v9.18.0...v9.19.0) Updates `github.com/spf13/pflag` from 1.0.9 to 1.0.10 - [Release notes](https://github.com/spf13/pflag/releases) - [Commits](https://github.com/spf13/pflag/compare/v1.0.9...v1.0.10) Updates `modernc.org/sqlite` from 1.47.0 to 1.50.0 - [Changelog](https://gitlab.com/cznic/sqlite/blob/master/CHANGELOG.md) - [Commits](https://gitlab.com/cznic/sqlite/compare/v1.47.0...v1.50.0) --- updated-dependencies: - dependency-name: github.com/jackc/pgx/v5 dependency-version: 5.9.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-and-patch - dependency-name: github.com/mark3labs/mcp-go dependency-version: 0.52.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: github.com/redis/go-redis/v9 dependency-version: 9.19.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch - dependency-name: github.com/spf13/pflag dependency-version: 1.0.10 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: go-minor-and-patch - dependency-name: modernc.org/sqlite dependency-version: 1.50.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: go-minor-and-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
xarmian
|
14e3e41185 |
chore: bump go to 1.26.3 + golang.org/x/net to v0.53.0 (TASK-1232) (#438)
Clears the 4 stdlib govulncheck findings that were blocking `make check`
on the previous baseline:
- GO-2026-4982 — meta content URL escaping XSS (html/template)
- GO-2026-4980 — Escaper bypass XSS (html/template)
- GO-2026-4971 — NUL byte panic on Windows (net)
- GO-2026-4918 — HTTP/2 SETTINGS_MAX_FRAME_SIZE infinite loop
(net/http) + golang.org/x/net needed v0.53.0
Changes:
- go.mod: `go 1.26.0` → `go 1.26.3` (minimum required Go version).
GOTOOLCHAIN=auto (the default) makes `go` auto-download 1.26.3 for
contributors still on an older local binary; CI's setup-go pin
`go-version: "1.26"` already floats to the latest 1.26.x patch and
needs no change.
- golang.org/x/net v0.52.0 → v0.53.0 (the GO-2026-4918 fix).
- golang.org/x/crypto v0.49.0 → v0.50.0, golang.org/x/sys v0.42.0 →
v0.43.0, golang.org/x/term v0.41.0 → v0.42.0 — pulled in as
cross-module compat partners by `go get golang.org/x/net@v0.53.0`.
Verification:
- govulncheck ./... → 0 vulnerabilities affecting our code
(1 unreached grpc finding GO-2026-4762 is
in the import graph only — separate task)
- golangci-lint run → 0 issues
- go test ./... → all pass
- cd web && npm run build → clean
- make check → fully green for the first time
Implements TASK-1232.
|
||
|
xarmian
|
48776a3967 |
feat(oauth): DCR + authorize + token endpoints + populated discovery (TASK-1025, sub-PR C of TASK-951) (#372)
* feat(oauth): DCR + authorize + token endpoints + populated discovery doc (TASK-1025, sub-PR C of TASK-951)
Third of 5 sub-PRs landing the OAuth 2.1 authorization server in
PLAN-943. Mounts the three flow-driving HTTP endpoints over the
fosite-backed server constructed in sub-PR B, replaces the
TASK-950 501 stub with the real RFC 8414 discovery doc, and ships
an inline-HTML consent stub as a TASK-952 placeholder so the
auth-code flow runs end-to-end.
What lands:
- internal/server/handlers_oauth.go (744 LoC)
- POST /oauth/register: RFC 7591 DCR. Hand-written, no fosite.
Public clients only (token_endpoint_auth_method=none rejected
for any other value), authorization_code + refresh_token
grants only, code response type only. Validates redirect_uris
(absolute, no fragment, https or loopback-http or custom-
scheme like claude://, blocks file:/javascript:/data:/vbscript:).
- GET /oauth/authorize: starts auth-code flow. fosite validates
request shape (PKCE-S256 required, audience matched, redirect
exact-match). If user has session → renders inline consent
stub. If not → 302 to /login?redirect=<self> (TASK-998's
plumbing in pad-cloud honors the redirect=).
- POST /oauth/authorize/decide: processes consent decision.
Form-bound CSRF token (the existing __Host-pad_csrf cookie,
read from a hidden form field instead of header). Approve →
fosite NewAuthorizeResponse → 303 to client.redirect_uri
with code. Deny → fosite WriteAuthorizeError(access_denied).
- POST /oauth/token: code + refresh exchange. fosite verifies
PKCE verifier (S256-required) + RFC 8707 audience. Returns
{access_token, token_type, expires_in, refresh_token, scope}.
RefreshTokenScopes=[] from sub-PR B means refresh ALWAYS
issues on authorize-code grant.
- Inline consent stub: minimal HTML form with Approve/Deny,
auto-grants every requested scope (TASK-952's UI replaces
with workspace allow-list selection per TASK-953).
- internal/server/handlers_well_known.go: handleOAuthAuthorizationServerStub
→ handleOAuthAuthorizationServer. Returns RFC 8414 metadata
with all six endpoint URLs (revoke + introspect URLs sub-PR D
fills with handlers; the URLs are stable now), advertised
scopes, S256-only code_challenge_methods,
resource_indicators_supported=true, authorization_response_iss_parameter_supported=true.
- internal/server/server.go: Server.oauthServer field +
SetOAuthServer + registerOAuthRoutes called from setupRouter
inside an r.Group with requireCloudMode + SessionAuth (so
/authorize can detect the logged-in user via __Host-pad_session;
SessionAuth falls through gracefully when no cookie).
- cmd/pad/main.go: oauthpkg.NewServer wired in cloud mode using
cfg.EncryptionKey as HMAC secret + cfg.MCPPublicURL+/mcp as
AllowedAudience. Wiring is conditional on PAD_MCP_PUBLIC_URL
being set (the OAuth surface needs a canonical audience to
bind tokens to).
CSRF posture: middleware_csrf.go runs only on /api/* paths so
/oauth/* is naturally exempt. The consent decision endpoint
adds its own form-token check (validateConsentCSRFToken) using
the same __Host-pad_csrf cookie the SPA uses, just with the
token in a hidden form field rather than a header. Same security
model, different transport.
Tests (12, all passing):
- TestOAuth_AuthorizationServerMetadata_PopulatedShape: pins
RFC 8414 metadata fields including S256-only PKCE +
resource_indicators_supported.
- DCR (5): happy path; missing redirect_uris; bad redirect-URI
shapes (relative, non-loopback http, fragment, javascript:);
non-public client auth method rejected; unknown grant type
rejected; not mounted outside cloud mode.
- /authorize (3): redirects to /login when no session;
renders consent stub when logged in; rejects audience
mismatch via fosite's audienceMatchingStrategy.
- /authorize/decide (2): rejects missing csrf_token; deny
produces access_denied redirect.
- Full PKCE flow: end-to-end /authorize/decide (approve) →
/token with code_verifier → 200 with access+refresh tokens.
- /token: rejects missing PKCE verifier.
Replaces the 501 stub assertion in TestMCP_AuthServerStub with
TestMCP_AuthServerMetadata_Mounted (just confirms 200; full
shape lives in the OAuth-handler test).
Out of scope:
- /oauth/revoke + /oauth/introspect (sub-PR D, TASK-1026)
- MCPBearerAuth OAuth introspection branch (sub-PR E, TASK-1027)
- Real consent UI with workspace allow-list (TASK-952)
* fix(oauth): translate RFC 8707 resource= to audience= + omit unmounted endpoints from discovery per Codex review (round 1)
Two findings from PR #372 round 1:
1. P1: Real RFC 8707 clients (Claude Desktop / Cursor / ChatGPT)
send `resource=` not `audience=`. fosite v0.49 reads only
`audience` from the form, so audienceMatchingStrategy was hit
with an empty needle and rejected every real-world authorize /
token request. Tests masked the gap by sending both keys.
Fix: translateResourceToAudience() copies r.Form["resource"]
into r.Form["audience"] before each handler invokes fosite.
Idempotent — if both keys are present, audience wins (test
harness sends both for belt-and-suspenders). Applied at
/authorize, /authorize/decide, and /token entry points.
Test TestOAuth_Authorize_AcceptsResourceOnly sends ONLY
resource= (no audience=) and asserts the request reaches the
consent stub. Without the translation it 303s with
invalid_request.
2. P2: /.well-known/oauth-authorization-server advertised
/oauth/revoke + /oauth/introspect endpoints that don't exist
yet (sub-PR D wires them). Real clients dialing those URLs
would get 404. RFC 8414 §2 lists revocation_endpoint +
introspection_endpoint as OPTIONAL, so omitting until the
handlers ship is spec-compliant + honest.
Fix: drop revocation_endpoint, introspection_endpoint, and
their *_endpoint_auth_methods_supported counterparts from
authServerMetadata. Sub-PR D's PR description includes
"populate these here" as a follow-up.
Test TestOAuth_AuthorizationServerMetadata_OmitsUnimplementedEndpoints
asserts the four fields are absent.
* fix(oauth): rate-limit /oauth/register + drop misleading iss flag per Codex review (round 2)
Two findings from PR #372 round 2:
1. P1: /oauth/register is open by RFC 7591 design (Claude Desktop /
Cursor self-register without prior auth) but had no rate limit.
An attacker could flood the oauth_clients table indefinitely.
Fix: extend RateLimit middleware to gate /oauth/register at
the same 5/hour/IP rate the existing /api/v1/auth/register
uses (RateLimiters.Register, burst 5). Added the OAuth route
group to the s.RateLimit middleware chain so the new path
actually runs through the limiter.
Other /oauth/* endpoints aren't rate-limited here: /authorize
rides session cookies (cheap to abuse but ineffective without
a logged-in user), /token is PKCE-bound to a stored code
(single-use), /authorize/decide is form-bound. Explicit per-
endpoint /oauth/* limits arrive with TASK-959.
Test TestOAuth_Register_RateLimited fires 5 requests
successfully, asserts the 6th returns 429.
2. P2: Discovery doc advertised
authorization_response_iss_parameter_supported=true, but the
/authorize success path delegates to fosite v0.49 which doesn't
add iss=<issuer> to the redirect. RFC 9207-aware clients seeing
the flag would treat the missing parameter as a protocol
violation.
Fix: drop the field from authServerMetadata. RFC 8414 §2
marks it OPTIONAL — omission is spec-compliant. We'll add
the parameter (+ post-processing of fosite's response) in a
future PR if a real client requires it; today's MCP clients
(Claude Desktop, Cursor, ChatGPT) don't.
Test TestOAuth_AuthorizationServerMetadata_OmitsUnimplementedEndpoints
extended to cover the field.
* fix(oauth): gate auth-server discovery doc on oauthServer != nil per Codex review (round 3)
Codex round 3 caught: /.well-known/oauth-authorization-server lives
in the MCP route group (registerMCPRoutes), while the /oauth/{
register,authorize,token} handlers live in the OAuth route group
(registerOAuthRoutes, gated on s.oauthServer != nil). A cloud
deployment with PAD_MCP_PUBLIC_URL unset gets MCP routes mounted
but NOT OAuth — the discovery doc would 200 with /oauth/* URLs
that 404. Worse for clients than no document at all.
Fix: handleOAuthAuthorizationServer now also nil-checks
s.oauthServer; on nil it returns 503 with config_error, matching
the existing fail-loud branch for when the issuer URL isn't
configured. Ops detect the misconfiguration immediately rather
than fielding "OAuth registration is failing with 404" tickets.
Test:
- TestOAuth_AuthorizationServerMetadata_503WhenOAuthDisabled
builds a Server with SetCloudMode + SetMCPTransport (so the
MCP route group mounts) but NOT SetOAuthServer; asserts the
endpoint returns 503 with config_error.
- TestMCP_AuthServerMetadata_Mounted renamed →
TestMCP_AuthServerMetadata_MountedAndGated to reflect the new
behavior under mcpEnabledTestServer (which doesn't wire OAuth).
The full 200 happy path lives in
TestOAuth_AuthorizationServerMetadata_PopulatedShape (uses
oauthEnabledTestServer).
* fix(oauth): apply gofmt to handlers_oauth_test + handlers_well_known
* fix(oauth): bump go-jose/v3 to v3.0.4 to resolve GO-2025-3485
CI govulncheck rejected the build: fosite v0.49.0 transitively
pulls github.com/go-jose/go-jose/v3@v3.0.3 which has
GO-2025-3485 (DoS in JWS parsing). Affected call site:
internal/server/handlers_oauth.go:408 — handleOAuthAuthorize calls
fosite.NewAuthorizeRequest which eventually calls jose.ParseSigned.
Fix: bump go-jose/v3 to v3.0.4 (the fixed version per the advisory).
go mod tidy auto-bumped dependent indirect deps too.
Verified locally:
govulncheck ./... → "No vulnerabilities found"
go test ./... → all green
go build ./... → clean
|
||
|
xarmian
|
f6eeee4f81 |
feat(oauth): fosite-backed authorization-server constructor (TASK-1024, sub-PR B of TASK-951) (#371)
* feat(oauth): fosite-backed authorization-server constructor (TASK-1024, sub-PR B of TASK-951)
Second of 5 sub-PRs landing the OAuth 2.1 authorization server in
PLAN-943. Wires fosite v0.49.0 over the storage layer from sub-PR A.
No HTTP routes yet — sub-PR C mounts /authorize, /token, /register;
sub-PR D mounts /revoke and /introspect.
What lands:
- internal/oauth/session.go — pad's *Session embedding fosite.DefaultSession
with typed UserID() accessor + Clone override returning *Session
(so handler-side type-assertions don't lose the concrete type
during refresh-token rotation).
- internal/oauth/storage.go — Storage adapter satisfying:
fosite.ClientManager
handler/oauth2.AuthorizeCodeStorage
handler/oauth2.AccessTokenStorage
handler/oauth2.RefreshTokenStorage
handler/oauth2.TokenRevocationStorage
handler/pkce.PKCERequestStorage
Compile-time guards in server.go assert each interface remains
satisfied. Translation: fosite.Requester ⇄ models.OAuthRequest
via JSON-encoded session_data + URL-encoded form. Sentinel errors
from sub-PR A map to fosite.ErrNotFound /
ErrInvalidatedAuthorizeCode / ErrInactiveToken.
- internal/oauth/audience.go — RFC 8707 custom AudienceMatchingStrategy.
fosite has no native RFC 8707; we close over a canonical audience
and reject any request that doesn't carry exactly that resource.
Belt-and-suspenders haystack check defends against fixtures /
migrations that register a client without setting Audience.
Plus ValidateAudienceParam (HTTP-handler entry helper) and
audienceForNewClient (DCR seed for sub-PR C).
- internal/oauth/server.go — NewServer(Config) → *Server returning
fosite.OAuth2Provider configured for:
- PKCE-S256 required (EnforcePKCE + EnablePKCEPlainChallengeMethod=false)
- Opaque HMAC tokens (compose.NewOAuth2HMACStrategy)
- Refresh rotation with grant-family revocation (sub-PR A round-2 fix)
- Audience binding via the custom strategy
Sensible default lifespans (1h access, 30d refresh, 15m authcode);
overridable via Config. Excluded by design: client-credentials,
implicit, ROPC (deprecated in OAuth 2.1), OpenID factories
(we're not OIDC), PAR (not needed for v1).
- go.mod — github.com/ory/fosite pinned at v0.49.0 (direct dep).
Tests (20):
- NewServer required-field validation (3) + default-lifespan path
- audienceMatchingStrategy: empty needle, mismatch, client-without-canonical,
canonical-only happy path, multi-audience rejection, no-canonical=ServerError
- ValidateAudienceParam (5 sub-cases) + audienceForNewClient
- Session: clone returns concrete *Session (not *DefaultSession);
nil-safe accessors
- Storage adapter: auth-code round-trip + invalidated-code error,
GetClient not-found mapping, access-token-inactive mapping,
rotation-revokes-entire-grant (end-to-end), PKCE round-trip,
requester-to-OAuthRequest session encoding + missing-client guard
Out of scope (subsequent sub-PRs):
- HTTP route handlers + DCR endpoint (sub-PR C / TASK-1025)
- /revoke + /introspect endpoints (sub-PR D / TASK-1026)
- MCPBearerAuth OAuth introspection branch (sub-PR E / TASK-1027)
* fix(oauth): inject canonical audience into hydrated clients per Codex review (round 1)
Codex round 1 caught a P1 in the storage adapter: modelClientToFosite
returned fosite.DefaultClient.Audience=nil for every persisted
client, but audienceMatchingStrategy's haystack-side check requires
client.GetAudience() to contain the canonical audience. Net result:
every authorize / token / refresh flow would fail with invalid_request
once the strategy ran, regardless of how the client was registered.
Fix: thread the canonical audience through Storage. NewStorage now
takes a canonicalAudience string; modelClientToFosite (now a method
on Storage) injects [canonicalAudience] into the hydrated client's
Audience field. The audience isn't persisted as a column —
single-resource AS for v1 (PLAN-943) means every client implicitly
allows the same audience, so storing what we'd always set to the
same value is pure write amplification.
Threading:
cfg.AllowedAudience → NewServer → NewStorage(store, audience)
└─ Storage.canonicalAudience
└─ modelClientToFosite injects
Misconfigured Storage (empty canonicalAudience — caught earlier by
NewServer's required-field check, but tests pin the fail-loud branch
in case Storage is ever constructed directly): produces clients
with Audience=nil so audienceMatchingStrategy rejects every request
with ServerError, surfacing the misconfiguration fast rather than
silently issuing wide-open tokens.
Tests:
- TestStorage_GetClient_InjectsCanonicalAudience — pins the
injection contract; without the fix this fails.
- TestStorage_NewStorage_EmptyCanonicalLeavesAudienceNil — pins the
fail-loud branch for misconfigured Storage.
- 6 existing tests updated to pass canonical audience to NewStorage
(mechanical sed update; behaviour unchanged).
* fix(oauth): hydrate request payload on inactive token Get*Session per Codex review (round 2)
Codex round 2 caught a HIGH-severity gap: GetRefreshTokenSession
returned (nil, fosite.ErrInactiveToken) for revoked rows, but
fosite's handleRefreshTokenReuse (flow_refresh.go:178-204) derefs
req.GetID() to drive the family revocation that's the OAuth 2.1
BCP §4.14 replay-detection rule. Returning nil nil-derefs that
flow and defeats replay detection — the very thing rotation exists
to enable.
Fix: hydrate the stored row even on the inactive path and return
(req, fosite.ErrInactiveToken). Mirrors the pattern already used
by GetAuthorizeCodeSession's invalidated-code branch. If
hydration itself fails (client deleted between issuance and use),
return the underlying error rather than masking it — replay
detection loses but the failure is observable.
Symmetric fix applied to GetAccessTokenSession even though
no fosite caller currently derefs on inactive there. Defense in
depth + uniform contract makes the adapter resilient to future
fosite changes (e.g. an introspector that wants req.GetID() for
audit-log enrichment).
Tests:
- TestStorage_GetRefreshTokenSession_InactiveReturnsPayload —
pins the refresh-side contract; without the fix this fails
on the nil-check.
- TestStorage_GetAccessTokenSession_InactiveReturnsPayload —
same pattern for access tokens.
* fix(oauth): set RefreshTokenScopes=[] so authorize-code grants issue refresh per Codex review (round 3)
Codex round 3 caught a P1: fosite defaults
Config.RefreshTokenScopes to ["offline", "offline_access"]. fosite
only mints refresh tokens when one of the listed scopes is granted.
PLAN-943's scope vocabulary is pad:read / pad:write / pad:admin —
no "offline" scope — so the default silently disabled refresh
issuance for every Pad grant, defeating the entire refresh-rotation +
family-revocation machinery this PR adds.
Fix: explicitly set RefreshTokenScopes: []string{} in NewServer's
fosite.Config. fosite reads the empty slice as "issue refresh on
every authorize-code grant whose client allows the refresh_token
grant type, no scope predicate" — matches fosite's own tests
(flow_authorize_code_token_test.go:129).
Pin: TestNewServer_RefreshTokenScopesIsEmpty documents the decision
+ smoke-checks that the constructor still returns a usable provider.
The actual "refresh issued on authorize-code grant" assertion lands
in sub-PR C's /token endpoint test — that's where fosite's
flow_authorize_code_token.go reads the field.
* fix(oauth): bump go.opentelemetry.io/otel{,/sdk} to v1.40.0 to resolve GO-2026-4394
CI govulncheck job rejected the build: fosite v0.49.0 transitively
pulls in go.opentelemetry.io/otel/sdk@v1.21.0 which has known
vulnerability GO-2026-4394 (Arbitrary Code Execution via PATH
Hijacking in go.opentelemetry.io/otel/sdk). Affected
init-time call sites:
internal/oauth/audience.go:8 → fosite.init → otel resource.init
internal/server/middleware_ratelimit.go:80 → sync.Once.Do → resource.Default
internal/cli/client.go:415,794,798 → otelhttp.* → trace.*
Fix: bump otel core + sdk + metric + trace to v1.40.0 (the fixed
version per GO-2026-4394's advisory). go mod tidy also pulled in
go.opentelemetry.io/auto/sdk@v1.2.1 as a new transitive.
Verified locally:
govulncheck ./... → "No vulnerabilities found"
go test ./... → all green
go build ./... → clean
|
||
|
xarmian
|
9905a83134 |
feat(mcp): pad mcp serve skeleton on stdio (TASK-944) (#333)
Stand up internal/mcp + the cobra `pad mcp serve` subcommand. v1 is
handshake-only — the server completes initialize and stays alive over
stdio, advertising tool capability with an empty registry. TASK-945
fills that registry from `pad help --format json`.
- New internal/mcp package wraps mark3labs/mcp-go's stdio transport;
graceful shutdown on EOF / SIGINT / SIGTERM / ctx-cancel.
- New cmd/pad/mcp.go registers `pad mcp` as a top-level cobra group
with the `serve` subcommand wired to internal/mcp.NewServer.
- 4 unit tests: NewServer construction, real initialize round-trip
(asserts serverInfo.name + version), fallback version locked,
graceful shutdown on ctx-cancel.
Live smoke: `echo '<initialize>' | pad mcp serve` returns
`serverInfo:{name:"pad-mcp",version:...}` with `tools:{listChanged:true}`.
cmdhelp emits the new command tree at `pad help mcp serve --format json`.
Parent: PLAN-942.
|
||
|
xarmian
|
cfda4463e8 |
feat(cmdhelp): tests + golden contract + drift validator (TASK-938) (#332)
* feat(cmdhelp): tests + golden contract + drift validator (TASK-938)
The verification layer that turns cmdhelp v0.1 from "implementation"
into "stable contract." Three categories of tests, all running in
`go test ./...`:
1. Schema validation (cmdhelp.schema.json as CI gate)
- internal/cmdhelp/schema_test.go — synthetic tree's emitted JSON
validates after static walk, after dynamic resolution, and after
a no-workspace fallback.
- cmd/pad/cmdhelp_real_test.go — the REAL pad cobra tree's emitted
JSON validates against the published schema. Future regressions
caught: types outside the closed vocabulary, non-numeric exit_code
keys, flag names violating propertyNames, malformed cmdhelp_version.
2. Drift-prevention contract (spec §6 / §11 Q5)
- internal/cmdhelp/example_validation.go — ValidateExamples walks
every example's `cmd` string, tokenizes with shellSplit, resolves
non-flag tokens against the live cobra tree, and asserts every
--flag exists on the resolved command (or any ancestor for
persistent / inherited flags). Negate-flag form (`--no-cache`)
is recognized via the negation rule from spec §5.3.
- shellSplit handles double/single quotes, backslash escape, and
stops at unquoted pipeline boundaries (|, ;, &, >, <) so the
validator only checks the first command in a pipeline.
- ValidateBoolArity asserts no bool flag appears in valued form
(--flag=value) anywhere in its examples (spec §5.3).
- cmd/pad/cmdhelp_real_test.go runs both validators against the
real pad tree as CI gates.
- Negative tests in internal/cmdhelp/example_validation_test.go
prove the validator catches: typo'd flag (--priorty), unknown
command path, valued-form bool flag.
3. Capabilities form equivalence (spec §8)
- cmd/pad/cmdhelp_real_test.go — both forms (help --capabilities
and --cmdhelp-capabilities fallback) produce byte-identical
output. Side-effect-free guarantee verified by passing garbage
args alongside the fallback flag.
Refactors enabling the tests:
- cmd/pad/main.go: extract newRootCmd() so tests can build the real
cobra tree without running it. main() body shrinks to two lines.
- cmd/pad/main.go: extract handleCmdhelpCapabilitiesFallback() so the
fallback's side-effect-free contract is directly assertable instead
of requiring a subprocess.
Parser improvements driven by real-pad-tree drift findings:
- parseExamplesFromLong: strip same-line `# comment` annotations so
`pad foo --bar # one item's attachments` doesn't pollute Examples.
stripCommentIndex is quote-aware (# inside "..." or '...' is literal).
- main.go (github cmd): the Long had annotations on example lines
separated only by spaces (no `#`), which was malformed input. Fixed
to use `#` separators — caught by the drift validator on first run.
New deps:
- github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 — Go JSON Schema
validator supporting draft 2020-12 (matches the cmdhelp schema's $schema).
New helpers in internal/cmdhelp/:
- FindAndCompileSchema(startDir) walks up to locate
schema/cmdhelp.schema.json and returns a compiled schema. Reusable
by any consumer that wants to validate cmdhelp documents.
End-to-end on real binary:
- pad help --format json → 100 commands, schema-valid.
- All examples in pad's emitted output resolve against the live tree
(zero drift findings).
- pad help --capabilities byte-identical to pad --cmdhelp-capabilities.
- Adding a typo'd flag in any cobra Long block in cmd/pad MUST break
TestRealPadTree_ExampleDriftValidator. Verified by the negative
TestValidateExamples_DetectsTypoFlag.
make check clean. All 53 cmdhelp + cmd/pad tests pass.
Parent: PLAN-930.
* fix(cmdhelp): pass full token stream to cobra.Find per Codex review (round 1)
Codex round 1 caught: ValidateExamples stopped collecting the command
path at the first flag, so an example like
pad --workspace foo item create task --priority high
resolved to root, not `item create`. That meant `--priority` was
checked against root's flag set (where it doesn't exist) — false
positive — AND the validator silently missed any command-path drift
after a leading root flag.
Cobra's own Find walks the full token stream and uses each command's
flag definitions to skip flag/value pairs while matching subcommand
names. Pass tokens[1:] directly to root.Find — let cobra handle the
interleaving correctly.
New test:
- TestValidateExamples_FlagBeforeSubcommandResolvesToCorrectTarget —
flag-before-subcommand resolves to the leaf and accepts leaf flags.
|
||
|
xarmian
|
9e8fec93ff |
fix(ci): bump golang.org/x/image to v0.39.0 to clear 5 govulncheck CVEs (#300)
govulncheck flagged 5 vulnerabilities reachable through the new attachments image processor (TASK-878), all in the golang.org/x/image module that disintegration/imaging pulls in transitively. We were stuck on the ancient v0.0.0-20191009234506-e7c1f5e7dbb8 because nothing else explicitly required a newer version. - GO-2026-4815: OOM from malicious IFD offset in tiff (fix v0.38.0) - GO-2024-2937: Panic on invalid palette-color images (fix v0.18.0) - GO-2023-1990: Excessive CPU on 0-height tiff images (fix v0.10.0) - GO-2023-1989: Excessive resource consumption in tiff (fix v0.10.0) - GO-2023-1572: DoS via crafted tiff image (fix v0.5.0) go get golang.org/x/image@latest landed v0.39.0, which fixes all five. golang.org/x/text bumped 0.35.0 → 0.36.0 as a transitive ride-along. Verification: go build ./... — clean go test ./... — pass govulncheck ./... — "No vulnerabilities found" This closes the last CI gap: PR #299 (gofmt + race-timeout) cleared the lint and PostgreSQL race-step failures; this clears the third red light. Race step on PR #299's merge run finished in 19m36s ✓ under the new 30m cap. |
||
|
xarmian
|
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
|
||
|
xarmian
|
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).
|
||
|
xarmian
|
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.
|
||
|
xarmian
|
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+" |
||
|
xarmian
|
a86cfb7cff |
feat(server): zxcvbn password strength check at registration / rotation / reset (TASK-669) (#193)
* feat(server): zxcvbn password strength check at registration / rotation / reset (TASK-669)
Previously all three entrypoints (bootstrap, register, password change,
password reset) only enforced 8 <= len <= 128. Top-of-breach-list
entries like "password", "password123", "qwerty1234", and "letmein1"
all passed that filter and could silently end up hashed into a real
account.
- New validatePasswordStrength helper wraps github.com/trustelem/zxcvbn
with:
* length guardrails (8-128) kept as cheap early exits
* user-input context (email, name) passed into the scorer so
Alice+"Alice2026" gets penalized as email-derived
* minimum score 2 (OWASP-recommended floor, "adequate for online
attack scenarios")
* empty context strings filtered — zxcvbn treats "" as a banned
substring which would incorrectly weaken every password
- Wired into all four validation points in handlers_auth.go:
bootstrap, register, PATCH /auth/me (password change), reset-password.
- Test suite uses a strong canonical password now
("correct-horse-battery-staple") so bootstrapFirstUser + login flows
don't fight the new check.
- Password_strength_test.go covers: length extremes, the RockYou
top-100 (password, 123456, qwerty, iloveyou, letmein1, …),
email-derived + name-derived patterns, and three acceptable
passphrases.
Parent: PLAN-643 (OSS Security Hardening).
* fix(server): use pending name/username as strength-check context in PATCH /auth/me (TASK-669)
Addresses Codex P2 on PR #193: a PATCH that changed BOTH name and
password used the OLD user.Name as the zxcvbn user-input context, so
a caller could rename themselves to Zaphod + set password "zaphodzaphod"
in one request and slip the identity-derived penalty.
- When input.Name/input.Username are set in the PATCH, use those
pending values (not user.Name / user.Username) as the context for
validatePasswordStrength. Email stays as user.Email — email change
has its own flow and confirmation, not inline here.
- TestPasswordChange_RejectsPasswordDerivedFromPendingName pins the
fix with an integration-level regression test.
- TestValidatePasswordStrength_ContextPenalizesDerivedPasswords pins
the underlying unit behavior (context string actually tips the
score) so a future library swap can't silently regress.
Parent: PLAN-643 (OSS Security Hardening).
* fix(server): identity-aware reset strength check + username context on registration (TASK-669)
Addresses two Codex comments on PR #193:
P2 — reset handler ran a context-less strength check because
ConsumePasswordReset was atomic and gave us the user only after the
token was burned. That made /auth/reset-password enforce a weaker
policy than bootstrap/register/rotation and opened an identity-derived-
password bypass on the primary recovery endpoint.
- New Store.LookupPasswordReset is a read-only validation that returns
the user without consuming the token. handleResetPassword now does
two-phase: lookup → strength-check with full context (email, name,
username) → consume. On strength rejection the token is NOT burned
so the user can try again on the same reset link instead of having
to request another email.
P3 — registration strength check only passed email and name, not the
caller-supplied username. Identity-derived passwords keyed on the
username alone slipped past the zxcvbn user-input penalty.
- Added input.Username as the fourth context arg to
validatePasswordStrength in /auth/register.
Tests:
- TestPasswordReset_UsesIdentityContext: weak identity-derived password
rejected; same token then accepts a strong one (token preserved).
- TestRegister_IncludesUsernameInStrengthContext: username passed to
strength check penalizes username-derived passwords.
Parent: PLAN-643 (OSS Security Hardening).
|
||
|
xarmian
|
0e32645bb5 |
feat: add TOTP two-factor authentication
Backend support for optional TOTP-based 2FA on user accounts:
- POST /auth/2fa/setup — generate TOTP secret, return QR code URI
- POST /auth/2fa/verify — verify code and enable 2FA with recovery codes
- POST /auth/2fa/disable — disable 2FA (requires password confirmation)
- POST /auth/2fa/login-verify — complete login with TOTP or recovery code
- Login returns {requires_2fa: true, user_id} when 2FA is enabled,
requiring a second step via /auth/2fa/login-verify
- 8 recovery codes generated on setup for account recovery
- User model extended with totp_secret, totp_enabled, recovery_codes
- Refactored user queries with shared scanUser/userColumns for DRYness
Implements TASK-169 under PLAN-15 (Pad Cloud: Hardening).
|
||
|
xarmian
|
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 |
||
|
xarmian
|
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) |
||
|
xarmian
|
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 |
||
|
xarmian
|
8aa6481421 |
PHASE-12: Security Hardening for Pad Cloud (#67)
* feat: enforce RBAC role checks on all mutation endpoints (TASK-150) Add requireMinRole helper and role enforcement to 30+ mutation handlers. Viewers are now blocked from all state-changing operations, editors can mutate items/docs/comments/views but not collections/webhooks/workspace settings, and only owners can perform administrative operations. Includes 11 integration tests with real auth covering viewer/editor/owner access across items, collections, documents, comments, agent roles, item links, and workspace operations. * fix: scope search results to user's workspaces (TASK-151) Search without a ?workspace= param previously returned results from all workspaces in the database. Now the handler resolves the authenticated user's workspace memberships and passes their IDs to the store query, ensuring results only include items from workspaces the user belongs to. Fresh installs (no users) retain unscoped search for backward compat. Includes integration test proving cross-workspace isolation. * fix: add webhook URL validation and SSRF protection (TASK-152) Webhook creation now validates URLs before accepting them: only HTTP(S) schemes allowed, embedded credentials rejected, private/reserved IPs blocked (loopback, RFC1918, link-local, cloud metadata 169.254.169.254), and hostnames are DNS-resolved to verify they don't point to private IPs. Defense-in-depth check also added to the dispatcher's deliver function so existing webhooks with unsafe URLs are blocked at delivery time. * feat: add CSRF protection with double-submit cookie pattern (TASK-153) Implements CSRF middleware that validates X-CSRF-Token header matches the pad_csrf cookie on all state-changing API requests. Bearer token auth, auth endpoints, and fresh installs are exempt. The frontend client reads the CSRF cookie and attaches the header on mutations. * feat: add per-endpoint rate limiting middleware (TASK-154) Adds IP-based rate limiting for auth endpoints (5/min login, 3/hr password reset, 5/hr registration) and user-based limits for API (100/min) and search (30/min). Uses golang.org/x/time/rate with automatic stale-entry cleanup. Adds chi RealIP middleware for correct client IP behind proxies. Returns 429 with Retry-After. * fix: sanitize error responses and remove PII from logs (TASK-155) Replace all writeError(500, err.Error()) calls with writeInternalError that logs the real error server-side and returns a generic message to clients. Remove email addresses, user IDs, and password reset tokens from log output to prevent PII leakage. * feat: add security headers, configurable CORS, and secure cookies (TASK-160) Add SecurityHeaders middleware (CSP, X-Frame-Options, nosniff, Referrer-Policy, Permissions-Policy). Make CORS origins configurable via PAD_CORS_ORIGINS env var. Add PAD_SECURE_COOKIES for TLS deployments (sets Secure flag on session/CSRF cookies and enables HSTS). Also adds X-CSRF-Token to CORS allowed headers. * fix: address PR review — lazy router init and trusted IP for rate limits Fix two issues flagged by Codex: 1. CORS/HSTS config was ignored because setupRouter() ran in New() before SetCORSOrigins/SetSecureCookies were called. Now uses sync.Once to lazily build the router on first ServeHTTP/Listen. 2. Rate limiter read X-Real-IP directly from untrusted headers, allowing clients to spoof IPs. Now uses RemoteAddr only (which chimiddleware.RealIP already sanitizes from trusted proxy headers). |
||
|
xarmian
|
46447e5504 |
feat: user management & authentication (Phase 6) (#14)
* feat: add user management database migration and models
Add migration 012_users.sql with users, sessions, and workspace_members
tables. Add user_id columns to api_tokens, items, comments, activities,
item_links, and item_versions for proper user attribution. Create Go
model structs (User, Session, WorkspaceMember) in models/user.go.
* feat: add store layer for users, sessions, and workspace members
Implement CRUD operations for user management:
- users.go: create, get, update, list, validate password (bcrypt)
- sessions.go: create, validate, delete, cleanup expired (SHA-256 hashed tokens)
- workspace_members.go: add/remove members, role management, access checks
Adds golang.org/x/crypto/bcrypt dependency. Includes 16 new tests
covering all store methods, password validation, session lifecycle,
and workspace membership operations.
* feat: rewrite auth system from single-password to user-based
Replace single-password auth with email/password user authentication:
- New endpoints: POST /auth/register, GET /auth/me
- Rewritten: POST /auth/login (email+password), GET /auth/session
(needs_setup detection), POST /auth/logout (DB session destroy)
- Delete in-memory SessionManager, use DB-backed sessions via store
- New middleware: SessionAuth (cookie→user), RequireAuth (with
fresh-install passthrough when no users exist)
- Remove Password field from config, PAD_PASSWORD env var, SetPassword()
All 23 existing server tests pass (fresh DBs have no users → passthrough).
* feat: add workspace access control middleware
Add RequireWorkspaceAccess middleware that checks workspace_members for
authenticated users, with fallback for legacy API tokens and fresh
installs (no users → implicit owner). Includes role hierarchy helpers
(workspaceRole, requireRole) for downstream permission checks.
Wire middleware into the /{slug} workspace route group.
* feat: add CLI auth commands and credential storage
Add pad login, pad logout, pad whoami commands with credential
storage in ~/.pad/credentials.json (0600 permissions). Update CLI
HTTP client to auto-attach auth tokens and X-Pad-Agent header on
all requests. Add auth API methods (Login, Register, Logout,
CheckSession, GetCurrentUser). Extend .pad.toml with optional
agent_name field. Add golang.org/x/term for masked password input.
* feat: derive actor/source from auth context in all handlers
Replace hardcoded "user"/"web" actor/source strings with auth-aware
helpers. actorFromRequest() derives actor ("user"/"agent" via
X-Pad-Agent header) and source ("web"/"cli" from auth method).
agentMeta() merges agent name into activity metadata. Update all
item, document, comment, and move handlers to use request-based
logActivity/logActivityWithMeta. Remove hardcoded CreatedBy/Source
from all CLI commands — server now determines these from auth context.
* feat: frontend auth — login, registration, auth guard, user menu
Rewrite login page with email/password fields, add registration page
for first-time setup, update auth guard to handle needs_setup redirect.
Add user menu to sidebar with logout. Update API client with new auth
methods (register, login with email, session with needs_setup flag).
* feat: migrate API tokens from workspace-scoped to user-owned
API tokens now have a user_id owner and optional workspace_id scope.
CreateAPIToken takes userID as first parameter. ValidateToken resolves
the token's user into the request context. TokenAuth middleware now
sets ctxCurrentUser when a user-owned API token is used. Add user-
scoped endpoints: GET/POST/DELETE /auth/tokens. Keep workspace-scoped
token endpoints for backwards compatibility.
* feat: workspace membership, invitations, and role enforcement
Add workspace_invitations table (migration 013) with join codes.
Implement invitation store methods (create, get by code, accept,
list). Add member management handlers: list members + invitations,
invite (auto-adds existing users or creates invitation), remove
member, change role, accept invitation by code. Add API routes
under /workspaces/{slug}/members/* and /invitations/{code}/accept.
Add CLI commands: pad members, pad invite, pad join.
* feat: auth tests and documentation updates
Add comprehensive auth endpoint tests: registration flow (first user
becomes admin), login/logout, validation errors, duplicate email,
auth enforcement (401 after users exist, exempt paths), /me endpoint.
Update CLAUDE.md and README.md to document user-based auth system,
replacing old PAD_PASSWORD references with pad login/members/invite
workflow and role-based access control.
* feat: add members management UI to workspace settings page
Add Members section to settings with: member list (avatar, name,
email, role), role change dropdown (owner only), remove button
(owner only), pending invitations display with join codes, and
invite form with email + role picker. Add members API methods to
the TypeScript client (list, invite, remove, updateRole).
* fix: backfill workspace owners for pre-migration workspaces
Add backfillWorkspaceOwners() that runs on server start. For any
workspace with no members, adds the first admin user as owner.
This handles the migration case where workspaces existed before the
user system — without it, the members list shows empty.
* feat: shareable invite links with /join/[code] page
Replace raw join codes with full shareable URLs. Server generates
join_url using its configured base URL (e.g. https://pad.example.com/
join/a3f8b2c1). New /join/[code] page handles the full flow: checks
auth → shows login/register if needed → accepts invitation → redirects
to workspace. Settings page shows "Copy invite link" button that copies
URL to clipboard. CLI outputs shareable link instead of raw code.
* fix: auto-add workspace creator as owner, integrate auth into pad init
handleCreateWorkspace now adds the authenticated user as owner of the
new workspace immediately — no more relying on the startup backfill.
pad init now checks auth status before making API calls. If no users
exist, prompts to register. If not logged in, prompts to login. After
auth, proceeds with workspace creation normally.
* fix: add join_url to invite response type in API client
* fix: address codex review — invite registration, logout token revocation, workspace scoping
- Allow registration with valid invitation_code (fixes invite flow for new users)
- Revoke Bearer session tokens on logout, not just cookies
- Filter workspace listing to user's memberships (admins see all)
|
||
|
xarmian
|
823afe615c |
feat: Add terminal colors and improved CLI formatting
Add fatih/color dependency for terminal color output. Status colors (green=done, yellow=in-progress, blue=open, red=cancelled), priority colors, item reference numbers in all list output, and colorized status icons throughout the CLI. |
||
|
xarmian
|
81579847c6 |
Initial release
Pad — project management for developers and AI agents. Single Go binary with embedded SvelteKit web UI, SQLite storage, CLI, and Claude Code /pad skill integration. https://getpad.dev |