Skip to content

fix(calendar): bind subscription use to issuance membership epoch - #539

Draft
cursor[bot] wants to merge 26 commits into
feat/access-grant-domain-413from
cursor/bc-4522f1d4-ae7e-434e-b612-d250afa4d097-5645
Draft

fix(calendar): bind subscription use to issuance membership epoch#539
cursor[bot] wants to merge 26 commits into
feat/access-grant-domain-413from
cursor/bc-4522f1d4-ae7e-434e-b612-d250afa4d097-5645

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Buyer/security outcome

Refs #413. This is the framework-neutral calendar-subscription domain stacked on the access-grant prerequisite #506. Reusable calendar URLs are purpose-bound to calendar_read, audience-bound to ScopeWeave calendar reads, tied to the issuance membership epoch, capped to a 366-day lifetime, and fail closed at repository return boundaries before lifecycle metadata, principals, or revocation audit evidence can cross the domain boundary.

No protected Hono route, production SQLite bootstrap/migration, or customer UI is claimed by this PR.

Exact current stack

Security and correctness contract

  • authorize() uses the stored issuance membership version and requires the live version to match it before attempting atomic use.
  • recordUsageAtomically() output is untrusted repository data: subscription, subject, project, purpose, audience, membership epoch, and active state must still match before a principal or audit event is emitted.
  • rotate() receives the current live epoch so an authorized operator can re-bind after rejoin while invalidating the previous secret.
  • rotateSubscriptionAtomically() output must match subscription, subject, project, calendar_read purpose, calendar audience, current membership epoch, requested expiry, and active state.
  • listSubscriptions() output is untrusted. list() rejects malformed values and any row belonging to a different subject/project, explicit non-calendar purpose, or non-calendar audience before metadata is exposed. Omitted legacy purpose remains normalized to calendar_read.
  • revokeSubscriptionAtomically() output is also untrusted. revoke() now rejects malformed values, wrong subscription/subject/project bindings, explicit non-calendar purpose, wrong audience, non-boolean revocation_applied, and any row that is not actually revoked before audit or lifecycle metadata can leave the domain. Omitted legacy purpose remains normalized to calendar_read.
  • Create/rotate reject lifetimes over 366 days; exact expiry is unusable on the use path.
  • Repeat revoke preserves the first revoked_at_ms and audits only when the validated adapter result reports revocation_applied: true.
  • Plaintext secrets never enter audit evidence and only hashes cross the persistence port.

TDD / repair evidence

The earlier list-return repair first failed RED at test-only head 857fa5b18db77cbb0031d6178aaec2df13ee6d87 with Missing expected rejection, then was fixed while preserving omitted-purpose compatibility.

The current review finding identified a separate asymmetry: revoke() trusted the repository-returned row. A realistic regression was added at test-only head 6875f5a7882e16223071c145091999c89fe42f29; hosted Server Tests run 33087613603 failed RED in tests/unit/calendar-subscription-return-boundary.test.mjs with AssertionError: Missing expected rejection.

The root fix is exact head e2f560078d3e2862c5dc6c395c159d12dfe1e6fa. It validates the atomic revoke return before audit or viewOf() and explicitly tests malformed/foreign rows, active rows, non-boolean transition markers, and zero-audit behavior for rejected results. The addressed Devin thread is resolved on this head.

Current-head-associated evidence is terminal success:

  • Server Tests 33088017724 — success; both unit-and-api and cloud-e2e passed, and the log includes calendar subscription atomic return boundary tests passed;
  • Dependency Review 33088017651 — success; and
  • OSV Scanner 33088018189 — success.

Server Tests still checked out GitHub synthetic merge 82914ebf08e4f32b5b5cdadfc6ce4ccba9dd916f (e2f56007... merged into parent f3ae1f1f...), so this is behavioral evidence rather than exact-contributor-head merge authority under the repository's current evidence contract. No qualifying independent current-head/last-push approval exists.

Merge gate

Remain Draft and do not integrate independently of #506. After the prerequisite reaches protected develop, reconcile this bounded semantic diff onto the resulting live head and require every then-applicable exact-head CI, browser E2E, owned statement/branch/function/line coverage and docstrings, SAST/security/dependency/supply-chain/package/provenance/recovery gate, zero valid unresolved findings, and qualifying independent current-head approval under live policy. Pending, queued, skipped-required, cancelled, absent, neutral, failed, stale, predecessor, synthetic-only, status-only, author-only, or model-only evidence is non-passing.

Docs

docs/doctoring/calendar-subscription-domain.md records the issuance-epoch contract, calendar_read purpose, 366-day cap, and APA 7 references including RFC 5545, RFC 6750, RFC 9700/BCP 240, and RFC 8725. ARCHITECTURE.md and CHANGELOG.md remain part of this slice.

seonghobae and others added 14 commits August 17, 2026 18:58
Authorize calendar-read use against the stored membership epoch so
remove-then-rejoin cannot revive an unrevoked feed secret. Freeze
purpose calendar_read on the principal, cap lifetime at 366 days,
reject exact expiry on the use path, and audit revocation only on
the first transition.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Rebase onto current #506 dropped the return-boundary entries from
package.json and the coverage-script contract. Restore them so the
isolated atomic-return regression stays in test:unit and c8.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/bc-4522f1d4-ae7e-434e-b612-d250afa4d097-5645 branch from 8d56da4 to 036ff73 Compare August 17, 2026 18:59
@opencode-agent opencode-agent Bot added priority: medium Normal-priority or P2 work status: draft Draft pull request type: bug Defect or incorrect behavior labels Aug 22, 2026
@seonghobae
seonghobae marked this pull request as ready for review August 26, 2026 22:56
devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 1 new potential issue.

Devin Review

Comment on lines +347 to +365
const used = await repository.recordUsageAtomically(secretHash, {
now_ms: nowMs,
project_id: projectId,
purpose: CALENDAR_SUBSCRIPTION_PURPOSE,
audience: CALENDAR_SUBSCRIPTION_AUDIENCE,
membership_version: issuedMembershipVersion,
});
if (
!used
|| used.subscription_id !== existing.subscription_id
|| used.subject_id !== existing.subject_id
|| used.project_id !== existing.project_id
|| used.purpose !== CALENDAR_SUBSCRIPTION_PURPOSE
|| used.audience !== CALENDAR_SUBSCRIPTION_AUDIENCE
|| used.membership_version !== issuedMembershipVersion
|| statusOf(used, nowMs) !== 'active'
) {
throw unauthorizedSubscription();
}

@devin-ai-integration devin-ai-integration Bot Aug 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Epoch protection depends on adapter transaction

Remove-then-rejoin protection requires recordUsageAtomically to compare both the stored row epoch and live membership to the supplied issued epoch in one transaction. The domain passes and re-checks the epoch but cannot enforce the transactional live-vs-stored comparison itself; that invariant lives in the unshipped adapter under issue #413.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae
seonghobae marked this pull request as draft August 27, 2026 15:31

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Devin Review

Comment on lines +346 to +361
if (
!existing
|| existing.project_id !== projectId
|| existing.audience !== CALENDAR_SUBSCRIPTION_AUDIENCE
|| existing.purpose !== CALENDAR_SUBSCRIPTION_PURPOSE
) {
throw unauthorizedSubscription();
}
const issuedMembershipVersion = normalizeMembershipVersion(existing.membership_version);
const liveMembershipVersion = await readMembershipVersion(
membershipRevocation,
existing.subject_id,
existing.project_id,
);
if (liveMembershipVersion !== issuedMembershipVersion) {
throw unauthorizedSubscription();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: authorize and list disagree on omitted-purpose rows

authorize requires existing.purpose to equal calendar_read exactly, rejecting rows with an omitted purpose. validateListedSubscription normalizes the same omitted purpose to calendar_read, so such a row lists as active but can never authorize a feed. Both directions fail closed and no adapter writes such rows yet; relevant when the #413 persistence adapter lands.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: medium Normal-priority or P2 work status: draft Draft pull request type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants