feat(calendar): add durable subscription lifecycle domain - #514
feat(calendar): add durable subscription lifecycle domain#514seonghobae wants to merge 11 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Do not merge this head. A reusable calendar URL that only re-checks live membership can survive remove-then-rejoin: authorize() captures a fresh version, the adapter compares that fresh value to live membership, and the stored issuance epoch is never consulted. That is acceptable for a ≤5-minute one-time grant; it is not acceptable for a feed secret that lives in calendar clients, browser history, and proxy logs.
Land the successor instead: #539 (03b0d91 on feat/access-grant-domain-413). It keeps this framework-neutral boundary and adds:
- issuance-epoch binding on use, with rotate as the rejoin re-bind path
- frozen
purpose: calendar_readon the principal so a thin route cannot treat the secret as a session-equivalent project credential - 366-day create/rotate lifetime cap and exact-expiry rejection on the use path
- first-transition-only revoke audit via
revocation_applied - exact-expiry, remove-then-rejoin, mid-check bump, rotate-vs-use, and leap-day tests
#524 (SQLite adapter) must rebase onto #539, not this head. Local npm run test:unit passed on #539. This review does not approve #514 or #539. Predecessor-head checks on a2ce027 are not authorizing evidence for #539.
Sent by Cursor Automation: Fix Issues
| const membershipVersion = await readMembershipVersion( | ||
| membershipRevocation, | ||
| existing.subject_id, | ||
| existing.project_id, | ||
| ); | ||
| const used = await repository.recordUsageAtomically(secretHash, { | ||
| now_ms: readNow(clock), | ||
| project_id: projectId, | ||
| audience: CALENDAR_SUBSCRIPTION_AUDIENCE, | ||
| membership_version: membershipVersion, | ||
| }); |
There was a problem hiding this comment.
This passes a fresh membership version into recordUsageAtomically(). After remove-then-rejoin the live version equals that freshly captured value, the unrevoked row still has the old epoch, and a leaked calendar URL keeps working.
Pass existing.membership_version (issuance epoch). Require live === issued && live === row.membership_version in the same transaction. Make revoke-on-membership-change mandatory, not an alternative. #539 implements this and adds a remove-then-rejoin test that expects 401 until rotate.
| subscriptionId: used.subscription_id, | ||
| subjectId: used.subject_id, | ||
| projectId: used.project_id, | ||
| audience: used.audience, |
There was a problem hiding this comment.
The authorized principal returns only audience. A thin route can treat this as a session-equivalent project principal and open JSON APIs, SSE, or attachments. Issue #413 requires project-only calendar read authority.
Freeze purpose: 'calendar_read' on create, persist, authorize, and the returned principal. #539 does this.
| h.setNow(1_800_000_005_000); | ||
| const principal = await h.service.authorize({ secret: created.secret, projectId: 'project-1' }); | ||
| assert.deepEqual(principal, { | ||
| subscriptionId: created.subscriptionId, | ||
| subjectId: 'user-1', | ||
| projectId: 'project-1', | ||
| audience: CALENDAR_SUBSCRIPTION_AUDIENCE, | ||
| }); | ||
| assert.deepEqual(h.usages, [{ subscription_id: created.subscriptionId, used_at_ms: 1_800_000_005_000 }]); |
There was a problem hiding this comment.
This authorizes at nowMs < expiresAtMs only. List-status at exact expiry in the edge file is not use-path coverage. Parent access-grant sets the clock to expiresAtMs and expects 401.
Add setNow(created.expiresAtMs) then authorize() → calendar_subscription_unauthorized. Also add a remove-then-rejoin case and a rotate-vs-use race where the old secret loses. #539 covers these.
|
Superseded by #539 after fresh exact-head comparison. #539 head |
1 participant
Buyer/security outcome
Refs #413. ScopeWeave's protected calendar feed still accepts a broad session JWT in the URL query string. Calendar clients need a reusable URL lifecycle that is materially different from the one-time
stream/attachment_viewgrant lifecycle in #506. This stacked slice adds the framework-neutral credential domain needed for a separately revocable, rotatable, project-bound calendar subscription without prematurely changing the protected route, schema, or UI.This PR does not close #413. It creates no database table or migration, changes no Hono route, issues no production subscription credential, and implements no UI. The issue #413 product-design prerequisite is captured in an editable Figma interaction contract; future UI implementation remains separate work and must conform to that contract plus the eventual API behavior.
Exact current stack
Protected
develop:28420da358f57be5e85be3660251e39b85e1cc94.Current parent/base:
feat/access-grant-domain-413@72460895916ce413146f111923fc05ebfae1dc43(#506).Current contributor head:
a2ce02710447ad4e46b57909f87d65fcfe5c1a10.Fresh parent→child comparison is ahead-only, zero-behind, has the exact current parent as merge base, and changes exactly seven files:
server/calendar_subscription_domain.mjstests/unit/calendar-subscription-domain.test.mjstests/unit/calendar-subscription-domain-edge.test.mjstests/unit/coverage-script-contract.test.mjspackage.jsondocs/doctoring/calendar-subscription-domain.mdCHANGELOG.mdNo existing auth/session implementation, Hono route, SQLite schema, migration ledger, Clearfolio behavior, attachment behavior, browser client, workflow, dependency, or lockfile is changed.
Parent reconciliation
The parent advanced from historical head
1c9d5cd6...to72460895...by absorbing the current protected Microsoft Project XML hardening. This branch was reconciled from the exact current parent tree, then only the six non-shared calendar-domain blobs were overlaid.CHANGELOG.mdwas merged semantically so the parent’s protected XML-whitespace and short-lived access-grant entries remain intact while the calendar-subscription entry is added.Current merge commit
a2ce0271...records the old child and exact current parent as ancestry, but the tree itself is parent-first rather than a stale-child reuse. Fresh comparison proves no protected XML/security/source regression and exactly the intended seven-file semantic delta. Any later parent movement requires the same parent-tree-first reconstruction and shared-file merge before this branch advances.Figma interaction contract
Editable design: https://www.figma.com/design/EwcePYFQ85DjBLhFfMXLvI
ScopeWeave Calendar Subscription Management — Issue 413specifies interaction/security, management list, create, one-time secret reveal, rotate confirmation, revoke confirmation, and empty states. The design is active-PR evidence, not shipped behavior; executable browser acceptance remains mandatory before a future UI slice can claim it.Implemented contract
scopeweave:calendar;csub_...correlation identifier;create()/rotate(); only SHA-256 hashes cross the repository port and neither plaintext nor hash enters audit events;TDD and verification boundary
The contract preceded production implementation:
c86c878f...imported an absent production module and failed RED withERR_MODULE_NOT_FOUND; subsequent commits supplied the domain and edge/failure coverage. Both focused behavior suites are registered in the canonical c8 path andcoverage-script-contract.test.mjslocks the registrations.Every workflow/check/review result predating
a2ce02710447ad4e46b57909f87d65fcfe5c1a10is historical and non-authorizing. Fresh exact-current-head repository and organization evidence is required. Pending, queued, skipped-required, neutral, stale, predecessor, synthetic, status-only, model-only, or author-only evidence is non-passing.Standards and integration boundary
docs/doctoring/calendar-subscription-domain.mddistinguishes protected truth from active stacked work, records the Figma contract, threat boundary, atomic membership contract, persistence/audit obligations, rollback, and APA 7 references to RFC 6750, RFC 9700/BCP 240, and RFC 8725.Do not integrate independently of #506. After #506 reaches protected
develop, reconcile this semantic slice onto the resulting live protected head and require every then-applicable exact-head repository/organization CI, browser E2E where applicable, owned production statement/branch/function/line coverage, docstring evidence, SAST/security/dependency/supply-chain gates, zero valid unresolved review findings, and qualifying current-head/last-push independent approval under the live rulesets. No predecessor-head evidence transfers.