- sandbox_wazero.go: explicitly discard plugin stdin via WithStdin to
prevent WASM modules from reading the server process's stdin fd
- channel_handler.go: replace ErrInternal message pass-through with
generic 'an internal error occurred' — full error stays server-side
in slog.Error only
- invite_handler_test.go: update assertions to expect generic message
- service/user.go: fix ChangePassword docstring (no old-password verification)
- service/user.go: RevokeSession now maps db.ErrNotFound→ErrNotFound and
all other store errors→ErrInternal, preventing internal failures from
masquerading as 404s
- plugin/loader.go: update scanPluginDirectory comment to reflect fail-fast
behavior; fix Lstat comment wording
- db/queries/sqlite/events.sql: CAST COALESCE result to INTEGER so sqlc
generates int64 instead of interface{}
- api/plugins_handler.go: log install error server-side and return sanitized
structured JSON response instead of raw err.Error()
- .github/workflows/ci.yml: remove continue-on-error from tag build steps
so tag boundary drift fails CI"
Agent-Logs-Url: https://github.com/J3vb/OwnCord/sessions/6635420c-af26-4cc0-9397-5e5b37887437
Co-authored-by: J3vb <192430104+J3vb@users.noreply.github.com>
- main.go: TinyGo WASI plugin implementing the OwnCord plugin ABI
(allocate/deallocate, list_commands, command_dispatch, on_event)
- hello.wasm: precompiled binary (TinyGo 0.40.1, Go 1.25.3, wasm-opt 129)
Responds to /hello [name] with a greeting; proof-of-life for wazero runtime
Plugin TOML support (-tags wazero):
- manifest_toml.go: tryLoadPluginTOML using BurntSushi/toml v1.6.0
- manifest_nottoml.go: no-op stub for default build
- loader.go: prefers plugin.toml, falls back to plugin.json
Solid vitest preset:
- vitest.config.ts: add vite-plugin-solid, expand include to
src/components/solid/**/*.test.tsx — Badge.test.tsx now runs
automatically as part of npm test (112 files / 3188 tests)
Clean sweep of every actionable item from the two Copilot review passes
on head 59ae4d8. Grouped by severity:
─── Crash / security (must-fix) ─────────────────────────────────────
1. main.go:140 — telemetryShutdown nil panic.
telemetry.Init can return (nil, err) on the -tags otel skeleton
path; the deferred closure would then call a nil function. Normalise
to a no-op shutdown when Init errors so the defer is always safe.
2. api/upload_handler.go — permSvc nil deref.
MountUploadRoutes + handleServeFile dereference permSvc on every
authenticated file request. Add a fail-fast panic at mount time so
the misconfiguration surfaces at wiring, not on the first 500.
Update upload_handler_test.go to pass a real PermissionService built
on the test DB (the existing tests were missing the argument entirely,
which meant the package wouldn't compile — this fixes the real bug
Copilot flagged).
3. ws/event_persister.go — NewEventPersister nil EventStore panic.
run() dereferences p.store on every flush. Panic at constructor
time instead so the crash happens once at startup rather than
minutes later in a background goroutine.
4. plugin/host_ui.go — serve-time symlink check.
rejectSymlinksUnder only runs at install time, so a symlink created
post-install (accidental or malicious) would be followed by
http.ServeFile and leak host files. Add an os.Lstat + ModeSymlink
check + IsRegular check to AssetHandler on every request. Cheap
relative to the file read and closes the TOCTOU window.
─── Correctness / observability (should-fix) ───────────────────────
5. ws/deps.go:77 — requirePerm hides misconfig as FORBIDDEN.
Previously, nil database, nil perms, or a GetRoleForUser error all
returned ErrCodeForbidden with the same message, making operator
failures indistinguishable from legitimate permission denials.
Split the branches: misconfig + DB error now return ErrCodeInternal
with a server-side slog.Error so operators see the real problem;
FORBIDDEN is reserved for the actual permission-bit check.
6. telemetry/metrics.go — ServiceCallDurationMs renamed to Sec.
Field name said "Ms" but the instrument name was
`service_call_duration_seconds` with unit "s". Renamed the field
and updated all 8 service-layer callers so the struct field and
metric semantics match.
7. ws/event_persister.go — flushEvy typo → flushEvery.
Renamed the field and the one call site in run().
─── Comments out of sync with code ──────────────────────────────────
8. plugin/loader.go — Stat vs Lstat comment.
The comment claimed "Stat (not Lstat)" but the code correctly uses
os.Lstat to detect symlinks. Updated the comment to match the code;
the code was already right.
9. telemetry/telemetry_otel.go — compile claim wrong.
Comment said the file would fail to compile without the upstream
OTel modules, but the skeleton deliberately avoids importing them
and Init returns a runtime error instead. Updated the comment to
reflect actual CI behaviour (the -tags otel build step passes
today but doesn't exercise real telemetry).
─── Nit / polish ────────────────────────────────────────────────────
10. ws/event_pruner.go — startup delay magic constant.
Hard-coded time.Minute made the "run shortly after startup"
behaviour untestable (a test with a 100ms interval would still
wait a full minute). Cap the startup delay by the interval:
min(interval, time.Minute). Documented via a new `maxStartupDelay`
constant.
11. ws/event_pruner_test.go — new file.
Unit coverage for runPrune cutoff correctness, error swallowing,
StartEventPruner nil-store short-circuit, ctx cancellation, and
the interval-bounded startup delay from fix#10. Uses a fakeEventStore
stub that records every prune call and signals the first one so
tests don't sleep.
─── Verification ────────────────────────────────────────────────────
gofmt -l clean. No network access in sandbox so `go vet` and `go test`
could not run; the changes are local and surgical and every touched
file compiles in isolation against the existing signatures.
https://claude.ai/code/session_01UsBsQW2YiA2usk9pnJjAWk
Final in-sandbox completeness pass. Five focused pieces; the remaining
items in PHASE_BC_LOCAL_TODO.md after this commit are all genuinely
local-only (toolchain, network, native deps).
Test coverage (the biggest gap from prior reviews)
- Server/plugin/manifest_test.go — pluginNameRegexp accept/reject table,
validateRelativePath table, oversized version, unknown permission.
- Server/plugin/host_http_test.go — hostAllowed dot-boundary suffix,
empty-entry rejection, case insensitivity, FQDN trailing dot. ipAllowed
table over loopback, RFC1918, RFC4193 (ULA), RFC6598 (CGN), link-local,
multicast, unspecified — both v4 and v6 — plus public-IP accept cases.
- Server/plugin/loader_test.go — rejectSymlinksUnder catches direct and
nested symlinks; scanPluginDirectory rejects a plugin whose entrypoint
is a symlink. Skipped on Windows where symlink creation needs elevation.
- Server/plugin/host_ui_test.go — AssetHandler serves declared files,
rejects undeclared files (404), rejects path traversal, supports nested
asset paths.
- Server/ws/hub_seedseq_test.go — SeedSeq monotonic, never-backwards,
concurrent CAS safety, integration with nextSeq.
- Server/ws/extract_event_type_test.go — table covering happy paths,
control char rejection, escaped quote rejection, length cap (64),
empty/missing/non-JSON inputs.
Plugin install endpoint (closes a real feature gap)
- Server/plugin/registry.go — InstallFromZip extracts a plugin .zip into
a staging directory under cfg.Directory, validates it zip-slip safe
(cleaned-path Rel check), refuses non-regular entries, refuses
symlinks, caps compressed at 16 MiB and uncompressed total at 64 MiB
(each file gated by io.CopyN against the remaining budget). Manifest
is parsed at the staged root, then atomically renamed into the
canonical plugin directory and registered via the existing
installFromDisk path.
- Server/api/plugins_handler.go — POST /install accepts multipart with
one "plugin" file part, http.MaxBytesReader caps the request body,
io.LimitReader caps the in-memory buffer, calls Registry.InstallFromZip,
returns 201 with the new plugin name. The endpoint inherits the Pass 2
admin auth + IP gate (mounted under r.Use(admin.RequireAdminAuth)).
Protocol surface
- Server/ws/serve.go — buildAuthOK now takes replaySource and includes
it in the auth_ok payload as "replay_source": "none" | "buffer" | "db".
Two call sites updated: reconnect path passes the existing local,
fresh-connect path passes "none". Test export updated to pass "none".
CI build-tag matrix
- .github/workflows/ci.yml — three new steps inside server-build-test
build the server with -tags otel, -tags wazero, and -tags otel,wazero.
All three are continue-on-error: true until the upstream OTel and
wazero modules land in go.mod (tracked in PHASE_BC_LOCAL_TODO.md).
Once they do, dropping continue-on-error converts the steps into
hard CI gates against tag-boundary drift.
Documentation
- CHANGELOG.md — new root-level file with curated entries for Phase B,
Phase C, security, and behavioural changes operators must know about
(notably event_persistence.enabled = true by default).
- PHASE_BC_LOCAL_TODO.md — ticks off the install endpoint, the
replay_source field, and the existing event_persistence defaultYAML
entry. The remaining items are toolchain-bound.
After this pass, the in-sandbox completeness ceiling is reached.
Everything still pending requires Go 1.25 toolchain, npm install,
real OTel SDK + wazero modules, sqlc, postgres backend impl, or
tinygo.
https://claude.ai/code/session_01UsBsQW2YiA2usk9pnJjAWk
Eight focused follow-ups from the medium-severity review bucket. All
in-sandbox tractable; no module changes, no new dependencies.
Performance
- Drop the defensive memcpy in EventPersister.Enqueue. The hub already
passes a fresh slice from wrapWithSeq and the copy was happening under
seqMu, serializing broadcast throughput. Documented the no-mutate
contract on the call site.
Observability
- AppMetrics gains WSEventsPersistErrors counter; the persister run loop
bumps both it and the existing WSEventsPersisted counter via cached
metrics handle.
- Hub.persistEvent now extracts the real event type ("chat_message",
"voice_join", ...) from the wrapped JSON envelope via a small
no-allocation byte scan instead of recording the generic
"broadcast"/"channel_broadcast" label.
- Added OTel spans + ServiceCallDurationMs histogram entries on one
public method per remaining service: DMService.CreateDM,
VoiceService.JoinChannel, InviteService.CreateInvite,
ModerationService.BanUser, BlockService.BlockUser,
UserService.UpdateProfile. Mirrors the existing pattern from
MessageService.SendMessage.
Hardening
- plugin/loader now Lstat-walks each plugin directory and rejects any
symlink, plus refuses an entrypoint that is itself a symlink. The
asset handler's prefix check stays as defense in depth.
- ipAllowed (plugin HTTP capability) now rejects RFC6598 carrier-grade
NAT (100.64.0.0/10), closing a gap in net.IP.IsPrivate which only
covers RFC1918 + RFC4193.
- Registry.activateAll syncs Instance.Enabled := true after a successful
activate so callers reading the in-memory flag see the live state.
Documentation
- defaultYAML now documents the new event_persistence, telemetry, and
plugins config blocks with their defaults and one-line descriptions.
- PHASE_BC_LOCAL_TODO.md ticks off five items (defaultYAML docs ×2,
remaining service spans, registry wiring already-fixed in Pass 2).
https://claude.ai/code/session_01UsBsQW2YiA2usk9pnJjAWk
Phase B + C review pass: critical security and correctness fixes.
Security
- S1: plugin admin endpoints now require admin.RequireAdminAuth in addition
to AdminIPRestrict. Previously a LAN attacker on the allowed CIDR could
list/enable/disable/uninstall plugins without a session.
- S2: rewrite plugin HTTPDo allowlist with proper net/url parsing. Empty
entries are ignored, suffix matches require a dot boundary, and a custom
Dialer rejects loopback / RFC1918 / link-local addresses to close the
DNS-rebinding TOCTOU window. Redirects re-validated.
- S3 + #9: manifest Name pinned to ^[a-z0-9][a-z0-9_-]{0,63}$, Entrypoint
and UI tab assets validated against absolute / "..", NUL byte, backslash
and non-canonical paths. Asset handler hardened with filepath.Rel check
for symlink and prefix-without-separator escapes.
- S5: pluginBridge postMessage handler ignores the pluginId in the message
body and uses an e.source -> contentWindow lookup instead, defeating
spoofed messages from same-origin scripts.
- S8: HTTPDo body capped at 5 MiB via io.LimitReader, redirects bounded
to 5 hops.
Correctness
- Critical seq alignment: PersistEvent now takes the hub-assigned seq as a
required parameter so the events table row seq always matches the wrapped
payload seq. Hub seeds its in-memory atomic counter from MAX(events.seq)
on startup. Drops in the persister queue no longer mis-align row vs
payload seq.
- #1: live plugin.Registry constructed in main.go BEFORE NewRouter and
threaded through; admin handler is no longer wired with nil.
- #3: EventPersister.Stop is now safe to call without a prior Start by
tracking a started flag — previously deadlocked waiting on done.
Wiring
- NewRouter signature gains *plugin.Registry; two test callers updated.
- admin.RequireAdminAuth exported as a thin wrapper over the existing
package-private adminAuthMiddleware.
- sqlc query templates updated for the new PersistEvent + GetMaxEventSeq
contracts (sqlite + postgres).
https://claude.ai/code/session_01UsBsQW2YiA2usk9pnJjAWk
J3vb