docs: fix ADR cross-references pointing at private decisions#12
Merged
Conversation
Public docs and one public ADR cited bare ADR-00xx numbers that resolve to the private internal/decisions set (and collide with the public docs/adr numbering). Drop or re-point those references so a public reader can't be misdirected into the private decision set: - docs/threat-model.md: drop bare private ADR refs (0009/0010/0012); re-point the isolate-secret-cache control to public ADR-0007; reword the .my ccTLD residual to describe the open diligence inline instead of a private ADR. - docs/adr/0007: drop the bare "(ADR-0009 §e)" private cross-reference, keeping the prose meaning. - .github/rulesets/README.md: replace the specific founder handle with a role and drop the literal actor_id value. Co-authored-by: Cursor <cursoragent@cursor.com>
choraria
force-pushed
the
chore/docs-adr-cross-refs
branch
from
June 12, 2026 22:03
20b4fa6 to
e5e0e04
Compare
choraria
added a commit
that referenced
this pull request
Jun 17, 2026
) Force patched versions of three dev-only transitive deps of @lhci/cli (the Lighthouse CI perf-gate tool in apps/www) via root pnpm.overrides, clearing 4 Dependabot alerts. @lhci/cli 0.15.1 is already latest, so it can't be bumped directly. None of these ship in any deployed Worker. Overrides (selective version-range keys, vulnerable ranges only): tmp@<0.2.6 -> ^0.2.6 (0.0.33/0.1.0 -> 0.2.7; #10, #12) uuid@<11.1.1 -> ^11.1.1 (8.3.2 -> 11.1.1; #11) js-yaml@<4.2.0 -> ^4.2.0 (3.14.2 -> 4.2.0; #13) The existing esbuild/postcss override pins (present in the committed lockfile but missing from package.json) are reinstated here so the package.json/lockfile reconcile leaves resolution byte-identical to main for everything outside the three target subtrees. Recon: the only major-bump risk is js-yaml v4 removing safeLoad, but the @lhci/utils call site is gated behind a .yml/.yaml extension check and our config is lighthouserc.json (JSON path), so safeLoad is never reached. uuid.v4() (no buffer arg) and tmp.fileSync/tmpNameSync are version-stable. Verified: full gate green + lhci autorun (5 runs) passes locally. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
choraria
added a commit
that referenced
this pull request
Jun 29, 2026
Public docs and one public ADR cited bare ADR-00xx numbers that resolve to the private internal/decisions set (and collide with the public docs/adr numbering). Drop or re-point those references so a public reader can't be misdirected into the private decision set: - docs/threat-model.md: drop bare private ADR refs (0009/0010/0012); re-point the isolate-secret-cache control to public ADR-0007; reword the .my ccTLD residual to describe the open diligence inline instead of a private ADR. - docs/adr/0007: drop the bare "(ADR-0009 §e)" private cross-reference, keeping the prose meaning. - .github/rulesets/README.md: replace the specific founder handle with a role and drop the literal actor_id value. Co-authored-by: Cursor <cursoragent@cursor.com>
choraria
added a commit
that referenced
this pull request
Jun 29, 2026
) Force patched versions of three dev-only transitive deps of @lhci/cli (the Lighthouse CI perf-gate tool in apps/www) via root pnpm.overrides, clearing 4 Dependabot alerts. @lhci/cli 0.15.1 is already latest, so it can't be bumped directly. None of these ship in any deployed Worker. Overrides (selective version-range keys, vulnerable ranges only): tmp@<0.2.6 -> ^0.2.6 (0.0.33/0.1.0 -> 0.2.7; #10, #12) uuid@<11.1.1 -> ^11.1.1 (8.3.2 -> 11.1.1; #11) js-yaml@<4.2.0 -> ^4.2.0 (3.14.2 -> 4.2.0; #13) The existing esbuild/postcss override pins (present in the committed lockfile but missing from package.json) are reinstated here so the package.json/lockfile reconcile leaves resolution byte-identical to main for everything outside the three target subtrees. Recon: the only major-bump risk is js-yaml v4 removing safeLoad, but the @lhci/utils call site is gated behind a .yml/.yaml extension check and our config is lighthouserc.json (JSON path), so safeLoad is never reached. uuid.v4() (no buffer arg) and tmp.fileSync/tmpNameSync are version-stable. Verified: full gate green + lhci autorun (5 runs) passes locally. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
choraria
added a commit
that referenced
this pull request
Jul 15, 2026
…ig 0082) (#616) * feat(web): member avatars on the Team page (dashboard-ux, follow-up) Founder request (#12): show each member's face next to their name on the Team page — it was name-only. Member identity already comes from the SECURITY DEFINER `org_member_directory()` (the `user` table is un-RLS'd), so migration 0082 widens that function to also return `image` (OAuth avatar URL) + `imageKey` (R2 upload), threaded through `OrgMember`. The face is served by a NEW, doubly-gated route `GET /api/org/{slug}/member-avatar/{userId}`: 1. `requireOrgAccess(slug)` — the REQUESTER must belong to the org (404s a non-member). 2. the target `userId` must appear in that org's member directory (co-membership) — else 404, so it can never fetch the avatar of someone you don't share an org with. Serving itself reuses the extracted, audited `serveAvatar` helper (R2-first → allowlisted provider/Gravatar proxy → forced `image/webp` + `nosniff` + `private`), now shared with `/api/avatar` (which is refactored onto it — behavior identical, its 11 route tests still green). `MemberAvatar` renders initials + the gated image (onError → initials, plus the pre-hydration zero-width check), wired into `team-manager` via `useOrgSlug()`. Human-UI: the avatars in the members table want a prod eyeball. Migration 0082 must be applied to prod before the web deploy. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TpcxGMYqQqhuDS1gy94ycT * refactor(db): drop unused image_key from member directory (review nit) Review flagged image_key as dead data returned from the SECURITY DEFINER org_member_directory(): the member-avatar serve path finds an uploaded avatar by the DETERMINISTIC R2 key from the userId (not a pointer), and only `image` (the provider URL) drives the provider-vs-Gravatar choice. Drop image_key from the definer + OrgMember to keep the definer's exposed surface minimal. (Perf finding — O(N) directory read per avatar — noted as an acceptable follow-up for large orgs.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TpcxGMYqQqhuDS1gy94ycT --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
choraria
added a commit
that referenced
this pull request
Jul 17, 2026
Second review round on slice 7. Root cause behind three findings: an explicitly
PRESENT but empty-string scoping value was silently dropped to "no constraint" at
the route's buildInput, before the contract could reject it — for endpointId that is
a silent SCOPE widening (a caller who meant one endpoint gets the whole org).
Fixed at the choke point + defense-in-depth:
- Canonical route buildInput: only an ABSENT `?endpointId` (not in the query at all)
means org-wide. A PRESENT-but-empty `?endpointId=` is passed THROUGH so the
contract's `uuid.optional()` 400s it — the SDK's `{ endpointId: "" }` lands here
as `?endpointId=` too, so this closes the SDK + raw-HTTP + Go paths at once (Go's
empty id now 400s instead of widening, no Go change needed). Contract test pins
`{ endpointId: "" }` / whitespace → VALIDATION_ERROR.
- TS SDK: `list`/`listPage` now also reject `{ endpointId: "" }` fail-fast (a clear
TypeError, no round trip), alongside the existing legacy-string-arg guard.
- CLI `--event-type ""` now errors (mirrors the endpointId positional + --search),
rather than forwarding `?eventType=` which the server drops to no-filter.
- CLI parity guard strengthened: it now asserts each facet flag's value is actually
FORWARDED as `?<facet>=` (via a capturing fetch), not merely parsed — a
registered-but-unwired future flag now fails instead of passing green.
openapi.json is UNCHANGED (buildInput is runtime-only), so the webhook-go re-vendor
(#12) stays valid. Suites: contract 52, openapi 24, sdk-ts 186, cli 835; typecheck 29.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RybDP88CyucJpg7hCbJVc9
choraria
added a commit
that referenced
this pull request
Jul 17, 2026
…hods, honest test title
Third review round on slice 7 — three findings:
- The TS SDK README's quickstart still called `events.list(endpoint.id)`, which now
throws with the new filters-first signature. Updated to `list({ endpointId })`.
(The docs.webhook.co Mintlify pages + the Python examples are slice-11 docs work —
updated once every SDK republishes to the canonical shape; the currently-published
Python SDK still has the old signature. webhook-go's README was fixed in its PR.)
- Adding `endpointId` to the shared EventsListFilters meant the deprecated
`listByEndpoint`/`listPageByEndpoint` (which take the id as a positional) would
SILENTLY ignore a `filters.endpointId` — scoping the wrong endpoint. They now throw
on an ambiguous `filters.endpointId`, and on an empty positional id (which would
otherwise build `/v1/endpoints//events`).
- routes.test title said "empty is dropped" while the assertion proves the opposite
(present-but-empty is passed through → 400). Corrected so a future reader can't
trust the title over the code and reintroduce the silent-widen bug.
sdk-ts 187, openapi 24. openapi.json unchanged (webhook-go #12 stays valid).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RybDP88CyucJpg7hCbJVc9
choraria
added a commit
that referenced
this pull request
Jul 17, 2026
…ias — four-surface parity (slice 7) (#649) * feat(api): canonical GET /v1/events (org-wide) + deprecated nested alias — four-surface parity Slice 7 of the org-wide events page: make events.list reachable ORG-WIDE across all four surfaces, not just per-endpoint, and canonicalize the route. Contract: `events.list` input `endpointId` is now OPTIONAL — absent = the org-wide browse (RLS-scoped), present = drill into one endpoint. This propagates to the MCP tool inputSchema automatically (verbatim shape) and to the CLI/API below. DB shared read-handler: branch on endpointId — absent → `listOrgEvents` (no existence gate, no endpoint-scoped headCursor); present → the existing `getEndpoint` NOT_FOUND gate + `listEvents` + headCursor. An unknown/other-org endpointId STILL 404s; a missing one no longer does. HTTP routes: `GET /v1/events?endpointId=` is now CANONICAL (operationId `eventsList`, org-wide by default); `GET /v1/endpoints/{endpointId}/events` stays as a DEPRECATED ALIAS (operationId `eventsListByEndpoint`) so published SDKs / hardcoded URLs keep working. RouteDef gained `alias`/`deprecated`/`operationId`; the generator honors them. routes.test's strict one-route-per-capability BIJECTION is now over CANONICAL routes only, plus an alias-integrity invariant (every alias names a capability with a canonical route, is deprecated, has its own operationId) and a generated-spec operationId-uniqueness check — strengthening the guard, not weakening it. CLI: `wbhk events list` endpointId is now an OPTIONAL positional (omit for the whole org); added the missing `--method` / `--dedup-strategy` / `--event-type` facet flags; the client now calls the canonical `/v1/events`. TS SDK: `events.list({ endpointId? })` / `listPage(...)` are canonical (org-wide capable) with the new facets; `listByEndpoint` / `listPageByEndpoint` are the deprecated nested-route methods (SDK spec-coverage ratchet updated for both). Parity guard: a new facet×surface matrix test asserts every contract filter facet is exposed as an API query param on both events routes — so a facet can't ship web/mcp only while 404ing on REST. MCP is structural (verbatim shape); CLI has a flag-per-facet test. OpenAPI regenerated (byte-match + generated-spec tests green); webhook-go re-vendor follows in a separate PR. Suites: contract 120, db 1442, openapi 40, api 213, cli 833, sdk-ts 184, mcp 112; full typecheck across 29 packages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RybDP88CyucJpg7hCbJVc9 * fix(sdk,cli): guard the org-wide migration footguns; widen the CLI parity guard Five findings from the slice-7 review: - TS SDK: `list`/`listPage` dropped the positional endpointId (now filters-first). A TypeScript caller gets a compile error, but an UNTYPED JS caller still passing `list('ep_123')` would have it silently bound to `filters` → a whole-org list, a silent data-scope widening on upgrade. Now a string first arg throws a TypeError with the migration (`list({ endpointId })`). - CLI: the endpointId positional became optional; `events list "$EP"` with an unset shell var would silently list the whole org. An explicitly-EMPTY endpointId is now a usage error; omitting it entirely stays the intended org-wide path. - Parity guard: the openapi matrix test only covered contract↔HTTP-route, but its comment claimed four-surface coverage. Corrected the comment, and added the CLI half in packages/cli events.test.ts: a facet→flag sample table asserted COMPLETE against the contract (a new facet with no entry fails) AND each flag EXERCISED (an unwired flag is a usage error). MCP stays structural (verbatim input shape). - browseEvents comment claimed a "7d default makes that rare" — that default is web-component-only; the shared handler now backs api/cli/mcp too, where the 5s statement_timeout is the ONLY universal backstop. Comment corrected to say so. - Stale JSDoc on the CLI api-client eventsList cited the deprecated nested route; updated to the canonical /v1/events (org-wide / ?endpointId=). Tests: sdk-ts 185 (+string-arg-rejected), cli 835 (+empty-arg + facet-parity), openapi 24, full typecheck 29. No behavior change to the org-wide read (RLS-scoped; security review was clean). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RybDP88CyucJpg7hCbJVc9 * fix: an empty endpointId must 400, never silently widen to the whole org Second review round on slice 7. Root cause behind three findings: an explicitly PRESENT but empty-string scoping value was silently dropped to "no constraint" at the route's buildInput, before the contract could reject it — for endpointId that is a silent SCOPE widening (a caller who meant one endpoint gets the whole org). Fixed at the choke point + defense-in-depth: - Canonical route buildInput: only an ABSENT `?endpointId` (not in the query at all) means org-wide. A PRESENT-but-empty `?endpointId=` is passed THROUGH so the contract's `uuid.optional()` 400s it — the SDK's `{ endpointId: "" }` lands here as `?endpointId=` too, so this closes the SDK + raw-HTTP + Go paths at once (Go's empty id now 400s instead of widening, no Go change needed). Contract test pins `{ endpointId: "" }` / whitespace → VALIDATION_ERROR. - TS SDK: `list`/`listPage` now also reject `{ endpointId: "" }` fail-fast (a clear TypeError, no round trip), alongside the existing legacy-string-arg guard. - CLI `--event-type ""` now errors (mirrors the endpointId positional + --search), rather than forwarding `?eventType=` which the server drops to no-filter. - CLI parity guard strengthened: it now asserts each facet flag's value is actually FORWARDED as `?<facet>=` (via a capturing fetch), not merely parsed — a registered-but-unwired future flag now fails instead of passing green. openapi.json is UNCHANGED (buildInput is runtime-only), so the webhook-go re-vendor (#12) stays valid. Suites: contract 52, openapi 24, sdk-ts 186, cli 835; typecheck 29. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RybDP88CyucJpg7hCbJVc9 * fix(sdk): fix the TS SDK README, guard the deprecated by-endpoint methods, honest test title Third review round on slice 7 — three findings: - The TS SDK README's quickstart still called `events.list(endpoint.id)`, which now throws with the new filters-first signature. Updated to `list({ endpointId })`. (The docs.webhook.co Mintlify pages + the Python examples are slice-11 docs work — updated once every SDK republishes to the canonical shape; the currently-published Python SDK still has the old signature. webhook-go's README was fixed in its PR.) - Adding `endpointId` to the shared EventsListFilters meant the deprecated `listByEndpoint`/`listPageByEndpoint` (which take the id as a positional) would SILENTLY ignore a `filters.endpointId` — scoping the wrong endpoint. They now throw on an ambiguous `filters.endpointId`, and on an empty positional id (which would otherwise build `/v1/endpoints//events`). - routes.test title said "empty is dropped" while the assertion proves the opposite (present-but-empty is passed through → 400). Corrected so a future reader can't trust the title over the code and reintroduce the silent-widen bug. sdk-ts 187, openapi 24. openapi.json unchanged (webhook-go #12 stays valid). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RybDP88CyucJpg7hCbJVc9 * fix(sdk): assertByEndpointArgs guards typeof before .trim() (friendly error for JS undefined) Fourth-review DX finding: assertByEndpointArgs called endpointId.trim() without a typeof-string guard, so an untyped JS caller passing undefined/null to listByEndpoint()/listPageByEndpoint() got a raw "cannot read .trim of undefined" instead of the friendly "endpointId must be a non-empty endpoint id" message its sibling validateEventsListArg produces for the equivalent mistake. Guard typeof first. Test covers undefined + null. sdk-ts 187 green. openapi.json unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RybDP88CyucJpg7hCbJVc9 * feat(sdk-python): canonical events.list org-wide + endpoint_id drill-down The Python SDK is hand-written with its own spec-coverage ratchet (twin of sdk-ts's), so slice 7's new canonical GET /v1/events route surfaced as a red `test (python)`: "spec routes with no SDK method and no exemption: ['GET /v1/events']". This mirrors the TS/Go split onto Python. `events.list` / `list_page` now hit the canonical `/v1/events` with `endpoint_id` as a keyword-only, optional drill-down (omit = org-wide), and carry the three new facets (method, dedup_strategy, event_type). The nested route lives on new deprecated `list_by_endpoint` / `list_page_by_endpoint` methods so the ratchet's `GET /v1/endpoints/{endpointId}/events` thunk still resolves. Shared filter query hoisted into `_query`; only endpointId placement differs by route. An empty endpoint_id emits `?endpointId=` (with_query drops None, keeps ""), which the route choke point forwards to contract validation → 400 — the same fail-fast the route fix gives every surface. 181 tests pass, 99.81% coverage (gate 95%). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RybDP88CyucJpg7hCbJVc9 * style(sdk-python): black line-wrap on the dedupStrategy ternary The format check (black==26.5.1) wraps the >88-col dedupStrategy line in the new _query helper. No behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RybDP88CyucJpg7hCbJVc9 --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docs/threat-model.mdanddocs/adr/0007-isolate-secret-cache-cryptokey-handles.mdso the public docs no longer point at decision records that aren't published (corrected stale ADR-0009/0010/0012 references; the isolate-cache refinement now points at ADR-0007 / drops the dangling pointers).actor_idin.github/rulesets/README.md("the org admin / sole maintainer" instead of naming@chorariaand exposingactor_id: 5), keeping the bypass-actor explanation accurate without leaking an individual identity or internal role id.Test plan
.myccTLD residual-risk line indocs/threat-model.mdand confirm it reads correctly without the private ADR pointer..github/rulesets/README.mdcopy (the "org admin / sole maintainer" phrasing and the removedactor_id) and confirm it's accurate and on-voice before merge.Made with Cursor