diff --git a/.changeset/share-link-eligibility-at-redemption.md b/.changeset/share-link-eligibility-at-redemption.md new file mode 100644 index 0000000000..31b30da671 --- /dev/null +++ b/.changeset/share-link-eligibility-at-redemption.md @@ -0,0 +1,61 @@ +--- +"@objectstack/plugin-sharing": minor +"@objectstack/spec": minor +--- + +fix(plugin-sharing,spec): hold `publicSharing.eligibility` at redemption, not only at mint (#13608) + +**BREAKING** runtime behaviour change on a published package: share links that +were legitimately minted can now stop resolving without anyone revoking them. +Shipped as `minor` under the repo's launch-window convention. + +`ShareLinkService.createLink()` evaluated the object's declared +`publicSharing.eligibility` predicate before writing a `sys_share_link` row, and +nothing evaluated it again. `resolveToken()` checked `revoked_at`, `expires_at`, +the audience gates, the password and record EXISTENCE — then served whatever +survived, under the system context, to a caller with no principal at all. So the +declaration read as a standing policy about which records may be reached +anonymously, while the platform held it at exactly one instant in a link's life. + +The state the predicate reads is the state an editor changes. Publish an article +`published` + `public`, mint a link, then flip `audience` to `internal` or +`status` back to `draft`: the object's own policy now says the record is not +eligible for link sharing, and the old token kept resolving and kept serving the +record in full. The remedy was to revoke every link on the record by hand, which +first requires knowing they exist. + +It also sat oddly beside its neighbour. In that same `resolveToken()`, the +record-existence probe is deliberately fail-CLOSED (an unanswered probe denies), +so a **deleted** record stopped being served immediately while a +**reclassified** one did not — two failure directions in one door. + +**What changed.** `resolveToken()` re-evaluates the predicate against the record +it is about to serve, through the same `assertEligible` the mint path calls, so +the two points cannot drift on strictness, on the declared-field binding, or on +which faults refuse. It is one read either way: when a predicate is declared the +existence probe's projection widens from `['id']` to the whole row instead of a +second query being issued, so an object with no `eligibility` key keeps the +exact probe it always had. Fail-closed, matching mint: a predicate that will not +compile, faults on the record, or answers anything other than `true` refuses. + +**The refusal is deliberately indistinguishable.** For a caller who may hold +nothing but a token, telling "does not exist" apart from "revoked" apart from +"no longer eligible" is an existence oracle, so the redemption refusal is the +same undifferentiated `null` a revoked, expired or unknown token already gets — +no new error code, no new response branch, and no usage stamp. Over HTTP an +ineligible link is answered with the generic `404 INVALID_OR_EXPIRED`, byte-for- +byte what a token that never existed receives. The readable reason a link died +is written to the server-side log instead. + +**Operator impact.** Deployments upgrading across this change can feel it +immediately: any live link whose record has since moved out of its object's +`eligibility` predicate stops resolving, with no revocation event and no grace +period. That is the intent — the alternative is a declared policy the platform +does not hold — but it is worth measuring before rollout: an object's +`eligibility` predicate is the thing to read, and the links at risk are those on +records that no longer satisfy it. An operator who needs such links to keep +working must widen the predicate; there is no per-link opt-out, deliberately. +`redactFields` behaviour, the audience/password gates and objects that declare +no `eligibility` key are all untouched and pinned. + + diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 2242da20b7..7d479f22ff 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -135,7 +135,7 @@ The largest single consumer — **20 of the 109 sites**. | 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) | | 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` | | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` | -| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:413`, `:467`, `:471`, `:544`, `:574` | +| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:423`, `:477`, `:481`, `:554`, `:584` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | | 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:157`, `:382` | diff --git a/content/docs/protocol/objectql/security.mdx b/content/docs/protocol/objectql/security.mdx index 0984480418..3d3231d6de 100644 --- a/content/docs/protocol/objectql/security.mdx +++ b/content/docs/protocol/objectql/security.mdx @@ -433,6 +433,7 @@ publicSharing: allowedPermissions: [view] redactFields: [internal_notes] maxExpiryDays: 30 + eligibility: "record.status == 'published'" ``` **Who may mint and revoke** (ADR-0111 D8). Minting a link requires the object's @@ -443,6 +444,29 @@ Revoking a link is allowed for the link's **creator**, a **record share-manager* `canManageShares`), or system context: a link someone else minted on your record is your record's exposure to kill, not only its creator's. +**When `eligibility` is enforced** (#13608). The optional `eligibility` CEL +predicate is a **standing policy about which records may be reached +anonymously**, not a mint-time check. The platform evaluates it when a link is +minted **and again on every redemption**, against the record it is about to +serve. So flipping a record out of the policy — `audience` to internal, `status` +back to draft — stops every token minted while it was in, immediately, with no +revocation step to remember. Fail-**closed** at both points: a predicate that +will not compile, or that faults on the record, refuses rather than assuming +consent. What an anonymous holder sees is the ordinary "invalid or expired" +answer, identical to the one an unknown token gets; the readable reason a link +died is written to the server log, never to the response. + + +**Upgrade note.** Before this, `eligibility` was enforced only at mint, so a +record reclassified after a link was issued kept being served in full to +anyone holding the URL. Deployments upgrading across that change can feel it: +links minted while a record qualified stop resolving as soon as it stops +qualifying. That is the intent — the alternative was a declared policy the +platform did not hold — but if you depend on links surviving a +reclassification, widen the predicate rather than relying on the old +behaviour. + + --- ## 6. Field-Level Encryption diff --git a/content/docs/references/data/object.mdx b/content/docs/references/data/object.mdx index 11ff6a09a3..1eb73bda3f 100644 --- a/content/docs/references/data/object.mdx +++ b/content/docs/references/data/object.mdx @@ -431,7 +431,7 @@ const result = ApiMethod.parse(data); | **allowedPermissions** | `Enum<'view' \| 'comment' \| 'edit'>[]` | optional | Permission levels selectable on the share dialog | | **maxExpiryDays** | `integer` | optional | Reject links with expiry beyond this many days | | **redactFields** | `string[]` | optional | Field names removed from records served via a share token | -| **eligibility** | `string` | optional | CEL expression that must evaluate to true on the target record | +| **eligibility** | `string` | optional | CEL expression that must evaluate to true on the target record. Enforced at BOTH points in a link's life: when the link is minted, and again on every redemption — a record that stops qualifying stops being served through links already minted for it. Fail-closed: a predicate that will not compile, or that faults on the record, refuses. | ### Nested Shape: `Object.actions[number]` diff --git a/packages/plugins/plugin-sharing/src/share-link-eligibility.test.ts b/packages/plugins/plugin-sharing/src/share-link-eligibility.test.ts index f78ad1fc82..908fe806a0 100644 --- a/packages/plugins/plugin-sharing/src/share-link-eligibility.test.ts +++ b/packages/plugins/plugin-sharing/src/share-link-eligibility.test.ts @@ -37,6 +37,27 @@ * `maxExpiryDays`) had no pin in this package before. That set is what makes * the ablation meaningful: with the new predicate removed, the eligibility * cases must flip red and these five must stay green. + * + * ## [#13608] The second seam: the same policy, held again at REDEMPTION + * + * Enforcing at mint alone left the adjacent half open, and the state the + * predicate reads is exactly the state an editor changes: publish an article + * `public`, mint a link, then flip `audience` to `internal` or `status` back to + * `draft`, and the token minted before the flip kept resolving — serving the + * record in full to a caller with no principal. The maintainer ruled + * (2026-08-31) that `resolveToken` re-evaluates the predicate before serving, + * fail-closed, and that the refusal reuses the undifferentiated `null` a + * revoked link already gets rather than inventing a distinguishable "no longer + * eligible" answer for an anonymous caller — that distinction is an existence + * oracle. + * + * So the redemption block below pins three things the mint block cannot: the + * reclassification repro on BOTH flips, the refusal's SHAPE (measured at the + * service seam and again at the HTTP seam an anonymous holder actually + * reaches), and the fail-closed arm asserted by its REASON rather than by its + * outcome — a predicate that cannot compile can never be merely false, and the + * reason it refuses is read off the server-side log, which is the only place + * the ruling leaves it. */ import { describe, it, expect, afterEach } from 'vitest'; @@ -46,7 +67,12 @@ import type { DriverQuery } from '@objectstack/spec/contracts'; // with this cannot accept a call the real engine refuses // (`check:engine-double-contract`). import { assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/objectql'; +import type { IHttpServer, IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; import { ShareLinkService } from './share-link-service.js'; +// [#13608] The PUBLIC seam an anonymous holder actually reaches. The refusal's +// shape is a claim about what that caller can observe, so it is measured there +// and not only on the service's return value. +import { registerShareLinkRoutes } from './share-link-routes.js'; import { SysShareLink } from './objects/sys-share-link.object.js'; /** @@ -94,6 +120,9 @@ afterEach(async () => { } }); +/** A refusal line the service wrote to its server-side log. */ +interface LoggedRefusal { msg: string; meta?: Record } + interface BootOptions { /** * The object definition the DRIVER is initialised from, when it must differ @@ -109,6 +138,12 @@ interface BootOptions { * own-key-`undefined` block for what is reproduced and why it is real. */ shapeRow?: (row: any) => any; + /** + * [#13608] Collect the service's server-side log. The redemption refusal is + * deliberately silent on the wire, so this is where the REASON it refused + * becomes assertable — and where the ruling says the reason belongs. + */ + logger?: { warn: (msg: string, meta?: Record) => void }; } /** @@ -129,9 +164,16 @@ async function boot(article: any = ARTICLE, options: BootOptions = {}) { for (const row of ROWS) await driver.create('article', row); const schemas: Record = { article, sys_share_link: SysShareLink }; + /** + * [#13608] Every read the service issues, in order. Two claims are measured + * off it: the `id`-only probe is UNCHANGED for an object with no predicate, + * and the eligibility path widens that same read rather than adding a second. + */ + const findCalls: Array<{ object: string; query: any }> = []; const engine = { getSchema: (name: string) => schemas[name], find: async (object: string, query: any) => { + findCalls.push({ object, query }); const rows = await driver.find(object, query); if (!options.shapeRow || object !== 'article' || !Array.isArray(rows)) return rows; return rows.map(options.shapeRow); @@ -148,8 +190,11 @@ async function boot(article: any = ARTICLE, options: BootOptions = {}) { return driver.update(object, dispatch.id as string, data); }, }; - const service = new ShareLinkService({ engine: engine as any }); - return { driver, service }; + const service = new ShareLinkService({ + engine: engine as any, + ...(options.logger ? { logger: options.logger } : {}), + }); + return { driver, service, engine, schemas, findCalls }; } /** Every row currently in the share-link table — the thing that must stay empty. */ @@ -547,3 +592,367 @@ describe('[#7861] publicSharing.eligibility is enforced at createLink', () => { }); }); }); + + +/** + * [#13608] The redemption seam. + * + * `resolveToken` checked `revoked_at`, `expires_at`, the audience gates, the + * password and record EXISTENCE — and served whatever survived, under + * `SYSTEM_CTX`, to a caller with no principal. The object's declared + * eligibility policy was consulted at mint and never again, so a record could + * be reclassified out of the policy and keep being published by a token minted + * while it was in. + */ +describe('[#13608] publicSharing.eligibility is enforced again at REDEMPTION', () => { + /** A token minted while `a_ok` still qualifies — the pre-condition of every case below. */ + async function mintOn(service: ShareLinkService, recordId = 'a_ok', extra: Record = {}) { + const link = await service.createLink( + { object: 'article', recordId, audience: 'public', permission: 'view', ...extra }, + CALLER, + ); + expect(link.token).toBeTruthy(); + return link; + } + + /** The row as the table holds it now — usage counters included. */ + async function linkRow(driver: SqlDriver, id: string): Promise { + const rows = await driver.find('sys_share_link', {} as DriverQuery); + return rows.find((r: any) => r.id === id); + } + + describe('THE REPRO — a record reclassified after mint stops being served', () => { + it('`audience` flipped to internal: the same token resolves, then does not', async () => { + const { driver, service } = await boot(); + const link = await mintOn(service); + + // Minted while eligible, and serving. + expect(await service.resolveToken(link.token, {})).not.toBeNull(); + + // The editor's flip — the exact step the card measured. + await driver.update('article', 'a_ok', { audience: 'internal' }); + + // Pre-fix this still returned the record in full to an anonymous caller. + expect(await service.resolveToken(link.token, {})).toBeNull(); + }); + + it('`status` reverted to draft: the same token resolves, then does not', async () => { + const { driver, service } = await boot(); + const link = await mintOn(service); + expect(await service.resolveToken(link.token, {})).not.toBeNull(); + + await driver.update('article', 'a_ok', { status: 'draft' }); + + expect(await service.resolveToken(link.token, {})).toBeNull(); + }); + }); + + /** + * Ruling item 2 (maintainer, 2026-08-31): the refusal REUSES the answer a + * revoked / expired link already gets. Distinguishing "does not exist" from + * "revoked" from "no longer eligible" for a caller with no principal is an + * existence oracle, judged the way this repo's `RESOURCE_NOT_FOUND` pins + * judge one. + * + * "It refuses" does not cover that, so the shape itself is asserted — twice, + * because the claim is about two different observers. + */ + describe('the refusal is the answer revoked / expired / unknown already give', () => { + it('at the service seam it is `null` — the identical value, not a lookalike', async () => { + const { driver, service } = await boot(); + + // ⚠️ The record-gone arm gets its OWN record. Deleting the reclassified + // one would satisfy this case through the #5190 probe instead, and the + // reclassification assertion would then pass with the whole eligibility + // gate ablated — measured, on the first run of this ablation. + await driver.create('article', { + id: 'a_second', + title: 'Another published + public', + status: 'published', + audience: 'public', + owner_id: 'u1', + }); + + const live = await mintOn(service); + const revoked = await mintOn(service); + const expired = await mintOn(service); + const gone = await mintOn(service, 'a_second'); + + await service.revokeLink(revoked.token, { isSystem: true } as any); + await driver.update('sys_share_link', expired.id, { + expires_at: new Date(Date.now() - 60_000).toISOString(), + }); + + // The reclassification, applied after every token above was minted. The + // record STAYS in the table — it is ineligible, not gone. + await driver.update('article', 'a_ok', { audience: 'internal' }); + // …and the other record is removed outright, for the #5190 arm of the + // same door. + await driver.delete('article', 'a_second'); + + const answers = { + reclassified: await service.resolveToken(live.token, {}), + revoked: await service.resolveToken(revoked.token, {}), + expired: await service.resolveToken(expired.token, {}), + recordGone: await service.resolveToken(gone.token, {}), + unknown: await service.resolveToken('zzzzzzzzzzzzzzzzzzzzzz', {}), + }; + + // Identity, not deep equality: every arm returns the SAME value, so + // there is nothing on the wire to tell them apart. + for (const [name, answer] of Object.entries(answers)) { + expect(answer, `${name} must answer with the shared refusal`).toBeNull(); + } + expect(Object.values(answers).every((a) => a === answers.revoked)).toBe(true); + }); + + it('the ineligible arm returns — it never throws a code an anonymous caller could read', async () => { + const { driver, service } = await boot(); + const link = await mintOn(service); + await driver.update('article', 'a_ok', { audience: 'internal' }); + + // `assertEligible` throws `RECORD_NOT_ELIGIBLE`; letting that escape here + // would hand the route a 422 with the policy's own text in it. + await expect(service.resolveToken(link.token, {})).resolves.toBeNull(); + }); + + /** + * The HTTP seam, driven end-to-end on the real service through the real + * route, with the route's SECURE default context — every request below is + * anonymous. + * + * What this measures, and why it is the honest reading of ruling item 2: + * the reclassified link lands in the route's generic "invalid / expired / + * revoked" answer, byte-for-byte the same one a token that NEVER EXISTED + * gets. It does not land in the 410 `EXPIRED_OR_REVOKED` branch, and it + * must not: the route picks that branch off `revoked_at` / `expires_at` on + * the row itself, so reaching it would require the service to hand the + * route a distinguishable "ineligible" answer — exactly what the ruling + * forbids. The bucket it does land in is the strictly LESS informative of + * the two: 410 would confirm to a holder that the token was real. + */ + it('at the HTTP seam an anonymous caller cannot tell it from an unknown token', async () => { + const { driver, service, engine } = await boot(); + const live = await mintOn(service); + const revoked = await mintOn(service); + await service.revokeLink(revoked.token, { isSystem: true } as any); + + const resolve = mountResolveRoute(service, engine); + + // Before the flip: the link serves the record. + const served = await resolve(live.token); + expect(served.status).toBe(200); + + await driver.update('article', 'a_ok', { audience: 'internal' }); + + const reclassified = await resolve(live.token); + const unknown = await resolve('zzzzzzzzzzzzzzzzzzzzzz'); + const revokedAnswer = await resolve(revoked.token); + + // The equality that IS ruling item 2 at this seam. + expect(reclassified).toEqual(unknown); + expect(reclassified.status).toBe(404); + expect(reclassified.body?.error?.code).toBe('INVALID_OR_EXPIRED'); + // Nothing about the policy, the predicate or the record reaches the wire. + expect(JSON.stringify(reclassified.body).toLowerCase()).not.toContain('eligib'); + expect(JSON.stringify(reclassified.body).toLowerCase()).not.toContain('audience'); + + // The pre-existing revoked bucket, recorded as measured rather than + // assumed: it is a DIFFERENT status, and this change does not move it. + expect(revokedAnswer.status).toBe(410); + expect(revokedAnswer.body?.error?.code).toBe('EXPIRED_OR_REVOKED'); + }); + + it('a refused redemption stamps no usage — exactly like a revoked one', async () => { + const { driver, service } = await boot(); + const link = await mintOn(service); + await driver.update('article', 'a_ok', { audience: 'internal' }); + + expect(await service.resolveToken(link.token, {})).toBeNull(); + + const row = await linkRow(driver, link.id); + expect(row.use_count ?? 0).toBe(0); + expect(row.last_used_at ?? null).toBeNull(); + }); + }); + + /** + * The fail-closed arm, asserted by the REASON it refuses. + * + * A case whose predicate is false anyway would pass with the whole + * fail-closed branch deleted, so each case here uses a record that WOULD + * qualify under the predicate it was minted with, and a predicate that + * cannot be false at all — it can only fault. The reason is read off the + * server-side log, which is the only place the ruling leaves it. + */ + describe('fail-closed: an unanswered predicate refuses, and says why only in the log', () => { + /** Mint under the working policy, then swap the object's predicate underneath the token. */ + async function mintThenBreakPredicate(eligibility: string) { + const logged: LoggedRefusal[] = []; + const { driver, service, schemas } = await boot(ARTICLE, { + logger: { warn: (msg, meta) => { logged.push({ msg, meta }); } }, + }); + const link = await mintOn(service); + + // The control: this token, this record, resolves under the declared + // policy. So a later refusal is about the SWAP, not about the record. + expect(await service.resolveToken(link.token, {})).not.toBeNull(); + + schemas.article = { ...ARTICLE, publicSharing: { ...ARTICLE.publicSharing, eligibility } }; + return { driver, service, link, logged }; + } + + it('a predicate that no longer compiles refuses a record that still qualifies', async () => { + const { service, link, logged } = await mintThenBreakPredicate('record.status ===== '); + + expect(await service.resolveToken(link.token, {})).toBeNull(); + + // The reason, and the only place it exists. `a_ok` is `published` + + // `public`, so no false verdict is available here — this refusal can + // only have come from the unevaluable arm. + expect(logged).toHaveLength(1); + expect(logged[0].meta?.reason).toBe('ELIGIBILITY_UNEVALUABLE'); + expect(logged[0].meta?.link).toBe(link.id); + expect(logged[0].meta?.record).toBe('a_ok'); + }); + + it('a predicate naming an UNDECLARED key refuses as a fault, not as a verdict', async () => { + const { service, link, logged } = await mintThenBreakPredicate("record.nope == 'x'"); + + expect(await service.resolveToken(link.token, {})).toBeNull(); + expect(logged[0].meta?.reason).toBe('ELIGIBILITY_UNEVALUABLE'); + }); + + it('a predicate resolving to a non-boolean has not consented', async () => { + const { service, link, logged } = await mintThenBreakPredicate('record.title'); + + expect(await service.resolveToken(link.token, {})).toBeNull(); + expect(logged[0].meta?.reason).toBe('RECORD_NOT_ELIGIBLE'); + }); + + it('the refusal reason names the two things an operator needs, and stays server-side', async () => { + const { service, link, logged } = await mintThenBreakPredicate('record.status ===== '); + await service.resolveToken(link.token, {}); + + expect(logged[0].msg).toContain('publicSharing.eligibility'); + expect(logged[0].meta?.detail).toContain('record.status ===== '); + }); + }); + + /** + * The other half of the guarantee: everything that was serving before must + * still serve, and the read must not have grown a second query. + */ + describe('nothing else moved', () => { + it('a record that stays eligible resolves exactly as before, redaction included', async () => { + const { driver, service } = await boot(); + const link = await mintOn(service, 'a_ok', { redactFields: ['title'] }); + + // A write that does NOT cross the policy — the record still qualifies. + await driver.update('article', 'a_ok', { title: 'Renamed, still published + public' }); + + const resolved = await service.resolveToken(link.token, {}); + expect(resolved).not.toBeNull(); + expect(resolved!.link.record_id).toBe('a_ok'); + // object default ∪ per-link, unchanged by this card. + expect(resolved!.redactFields).toEqual(['owner_id', 'title']); + + const row = await linkRow(driver, link.id); + expect(row.use_count).toBe(1); + expect(row.last_used_at).toBeTruthy(); + }); + + it('an object with NO eligibility key is untouched by a reclassification', async () => { + const noEligibility = { ...ARTICLE, publicSharing: { ...ARTICLE.publicSharing, eligibility: undefined } }; + const { driver, service } = await boot(noEligibility); + const link = await mintOn(service); + + await driver.update('article', 'a_ok', { audience: 'internal', status: 'draft' }); + + // No declared policy, so nothing to hold: the link keeps serving. + expect(await service.resolveToken(link.token, {})).not.toBeNull(); + }); + + it('the existence probe keeps its `id`-only projection when no predicate is declared', async () => { + const noEligibility = { ...ARTICLE, publicSharing: { ...ARTICLE.publicSharing, eligibility: undefined } }; + const { service, findCalls } = await boot(noEligibility); + const link = await mintOn(service); + + findCalls.length = 0; + expect(await service.resolveToken(link.token, {})).not.toBeNull(); + + const articleReads = findCalls.filter((c) => c.object === 'article'); + expect(articleReads).toHaveLength(1); + expect(articleReads[0].query.fields).toEqual(['id']); + }); + + it('the eligibility read WIDENS the same probe rather than adding a second query', async () => { + const { service, findCalls } = await boot(); + const link = await mintOn(service); + + findCalls.length = 0; + expect(await service.resolveToken(link.token, {})).not.toBeNull(); + + const articleReads = findCalls.filter((c) => c.object === 'article'); + expect(articleReads).toHaveLength(1); + expect(articleReads[0].query.fields).toBeUndefined(); + }); + + it('[#5190] a deleted record still refuses — the probe survived the refactor', async () => { + const { driver, service } = await boot(); + const link = await mintOn(service); + await driver.delete('article', 'a_ok'); + + expect(await service.resolveToken(link.token, {})).toBeNull(); + }); + + it('[#5190] a deleted record refuses even with NO predicate declared', async () => { + const noEligibility = { ...ARTICLE, publicSharing: { ...ARTICLE.publicSharing, eligibility: undefined } }; + const { driver, service } = await boot(noEligibility); + const link = await mintOn(service); + await driver.delete('article', 'a_ok'); + + expect(await service.resolveToken(link.token, {})).toBeNull(); + }); + }); +}); + +/** + * [#13608] Mount the real PUBLIC resolve route on the real service. + * + * Only the verbs `registerShareLinkRoutes` calls are implemented, and the + * SECURE default `contextFromRequest` is deliberately left in place: it reads + * no identity header, so every request driven through the returned function is + * anonymous — the caller the refusal shape is a claim about. + */ +function mountResolveRoute(service: ShareLinkService, engine: unknown) { + const routes = new Map(); + const http: any = { + get: (path: string, h: RouteHandler) => { routes.set(`GET ${path}`, h); return http; }, + post: (path: string, h: RouteHandler) => { routes.set(`POST ${path}`, h); return http; }, + put: (path: string, h: RouteHandler) => { routes.set(`PUT ${path}`, h); return http; }, + delete: (path: string, h: RouteHandler) => { routes.set(`DELETE ${path}`, h); return http; }, + patch: (path: string, h: RouteHandler) => { routes.set(`PATCH ${path}`, h); return http; }, + use: () => http, + listen: async () => undefined, + close: async () => undefined, + getInstance: () => null, + }; + registerShareLinkRoutes(http as IHttpServer, service, engine as any); + + const handler = routes.get('GET /api/v1/share-links/:token/resolve'); + if (!handler) throw new Error('the public resolve route was not mounted'); + + return async (token: string): Promise<{ status: number; body: any }> => { + const captured: { status: number; body: any } = { status: 200, body: undefined }; + const res: any = { + status: (code: number) => { captured.status = code; return res; }, + json: (data: any) => { captured.body = data; return res; }, + send: () => res, + header: () => res, + }; + const req: any = { params: { token }, query: {}, headers: {}, method: 'GET', path: '/' }; + await handler(req as IHttpRequest, res as IHttpResponse); + return captured; + }; +} diff --git a/packages/plugins/plugin-sharing/src/share-link-service.ts b/packages/plugins/plugin-sharing/src/share-link-service.ts index c003345a9e..3909d07fea 100644 --- a/packages/plugins/plugin-sharing/src/share-link-service.ts +++ b/packages/plugins/plugin-sharing/src/share-link-service.ts @@ -231,12 +231,22 @@ async function defaultVerifyPassword(password: string, hash: string): Promise([...(policy.redactFields ?? []), ...((row.redact_fields as string[]) ?? [])]), ); @@ -653,32 +706,99 @@ export class ShareLinkService implements IShareLinkService { } /** - * [#5190] Is `(object, recordId)` still there? Read under the SYSTEM context - * on purpose: the question is EXISTENCE, not the holder's visibility — the - * token is the authorisation, and an anonymous holder has no context to read - * under in the first place. + * [#5190 / #13608] Read the shared record at redemption time: the existence + * probe, and — when the object declares an eligibility predicate — the row + * that predicate is judged on. `null` means "do not serve". + * + * Read under the SYSTEM context on purpose: the questions are EXISTENCE and + * the object's own POLICY, not the holder's visibility — the token is the + * authorisation, and an anonymous holder has no context to read under in the + * first place. * * Fails CLOSED. A probe that throws (driver blip, unregistered object) is an * unanswered question, and an unanswered question must not authorise: the - * caller treats `false` exactly like revoked. Note this is the OPPOSITE + * caller treats `null` exactly like revoked. Note this is the OPPOSITE * direction from the orphan sweep, which leaves rows alone when its probe * fails — and for the same principle. Neither acts on an unanswered question; * for a grant the safe direction is "deny", for a deletion it is "keep". + * + * [#13608] `withRecord` widens the projection from `['id']` to the whole row + * rather than issuing a second query, and it widens ONLY when a predicate is + * there to read it: an object with no `eligibility` key keeps the exact + * `id`-only probe it has always had. */ - private async recordStillExists( + private async loadRecordForServing( object: string | null | undefined, recordId: string | null | undefined, - ): Promise { - if (!object || !recordId) return false; + withRecord: boolean, + ): Promise | null> { + if (!object || !recordId) return null; try { const rows = await this.engine.find(String(object), { where: { id: recordId }, - fields: ['id'], + ...(withRecord ? {} : { fields: ['id'] }), limit: 1, context: SYSTEM_CTX, } as any); - return Array.isArray(rows) && rows.length > 0; + const found = Array.isArray(rows) ? rows[0] : undefined; + return (found as Record | undefined) ?? null; } catch { + return null; + } + } + + /** + * [#13608] The redemption half of the eligibility gate. + * + * ## It is the MINT gate, called again + * + * The verdict comes from `assertEligible` itself, not from a second + * evaluation written to match it. Mint and redemption therefore cannot drift + * on strictness (`true`, never merely truthy), on the declared-field binding, + * or on which faults refuse: there is one implementation and both seams call + * it. + * + * ## Why the answer collapses to a boolean here (ruling item 2, 2026-08-31) + * + * `assertEligible` throws two distinct codes on purpose — + * `RECORD_NOT_ELIGIBLE` (this record does not qualify) and + * `ELIGIBILITY_UNEVALUABLE` (the predicate is broken for every record until + * its author fixes it) — because at MINT they reach a caller who has already + * proved they can see the record, and the distinction tells them which thing + * to fix. + * + * At REDEMPTION the caller may hold nothing but the token. Telling such a + * caller "does not exist" apart from "revoked" apart from "no longer + * eligible" is an existence oracle, judged here the same way the + * `RESOURCE_NOT_FOUND` family is. So every refusal collapses into the one + * answer revoked, expired and record-gone already give: `null` — no code, no + * distinct error, nothing an anonymous holder can read a policy out of. + * + * The human-readable "why this link died" is not discarded; it goes to the + * server-side log, which is where the ruling puts it. + */ + private stillEligible( + eligibility: string, + record: Record, + schema: any, + row: ShareLink, + ): boolean { + try { + assertEligible(eligibility, record, schema, String(row.object_name)); + return true; + } catch (err: any) { + this.logger?.warn?.( + '[share-link] redemption refused — the record is no longer eligible under publicSharing.eligibility', + { + link: row.id, + object: row.object_name, + record: row.record_id, + // The distinction the anonymous response deliberately withholds. It + // is readable HERE, and only here. + reason: err?.code ?? 'UNKNOWN', + detail: err?.message, + }, + ); return false; } } diff --git a/packages/spec/src/contracts/share-link-service.ts b/packages/spec/src/contracts/share-link-service.ts index 53ae37ee73..d2bc3a85b7 100644 --- a/packages/spec/src/contracts/share-link-service.ts +++ b/packages/spec/src/contracts/share-link-service.ts @@ -36,6 +36,16 @@ * whole `resolveAuthzContext` envelope, not a per-site subset. See * {@link ShareLinkExecutionContext} for the boundary this draws and * why (#6206 / #6430). + * + * 6. **`publicSharing.eligibility` is a STANDING policy, not a mint-time + * check (#13608).** Implementations evaluate the object's declared + * predicate when the link is minted AND again on every `resolveToken`, + * against the record about to be served. A record that stops qualifying + * stops being served through tokens minted while it did qualify — no + * revocation step, no grace period. Fail-closed at both points: a + * predicate that cannot be evaluated refuses. The redemption refusal is + * the undifferentiated `null` documented on {@link + * IShareLinkService.resolveToken}. */ import type { ExecutionContext } from '../kernel/execution-context.zod.js'; @@ -217,9 +227,20 @@ export interface IShareLinkService { listLinks(filter: ListShareLinksFilter, context: ExecutionContext): Promise; /** - * Resolve a token at request-handling time. Returns null when the - * token does not exist, is revoked, expired, or fails the audience - * check. Increments `use_count` / `last_used_at` as a side effect. + * Resolve a token at request-handling time. Increments `use_count` / + * `last_used_at` as a side effect of a SUCCESSFUL resolution only. + * + * Returns `null` when the token does not exist, is revoked, is expired, + * fails the audience or password gate, names a record that no longer exists + * (#5190), or names a record that no longer satisfies the object's + * `publicSharing.eligibility` predicate (#13608). + * + * ⛔ That single `null` is the contract, not an implementation detail. The + * caller of this method may hold nothing but a token, and distinguishing + * "does not exist" from "revoked" from "no longer eligible" for such a + * caller is an existence oracle. Implementations MUST NOT return a + * distinguishable answer per reason, and MUST NOT throw one either; the + * readable reason belongs in the server-side log. * * @param token raw token from the URL / cookie * @param probe contextual gates the caller has already evaluated diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index f0e6512b72..71a34b4517 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -2250,12 +2250,32 @@ const ObjectSchemaBase = strictObject( */ redactFields: z.array(z.string()).optional().describe('Field names removed from records served via a share token'), /** - * Optional CEL/JSONLogic predicate evaluated against the candidate - * record when a link is created. When the predicate returns false, - * the create call fails with 422 (e.g. "draft records cannot be - * shared"). Evaluator is the same one used by sharing rules. + * Optional CEL predicate over the candidate record. It is a STANDING + * policy about which records may be reached anonymously, and the platform + * holds it at BOTH points in a link's life (#13608): + * + * - **at mint** — `createLink` refuses with 422 when the predicate is + * false (e.g. "draft records cannot be shared") and writes no link row; + * - **at redemption** — every `resolveToken` re-evaluates it against the + * record it is about to serve. A record that STOPS qualifying (an + * `audience` flipped to internal, a `status` reverted to draft) stops + * being served through links minted while it did qualify. + * + * ⚠️ Tightening this policy therefore cuts off already-minted links, on + * purpose — no revocation step, no grace period. That is the point of a + * standing policy, and it is a behaviour change for deployments that + * shipped before #13608. + * + * Fail-CLOSED at both points: a predicate that does not compile, that + * faults on the record, or that answers anything other than `true` refuses + * rather than assuming consent. The redemption refusal is deliberately + * INDISTINGUISHABLE from a revoked, expired or unknown token — an + * anonymous holder is told nothing about why a link died; that reason goes + * to the server-side log. + * + * Evaluator is the same one used by sharing rules. */ - eligibility: z.string().optional().describe('CEL expression that must evaluate to true on the target record'), + eligibility: z.string().optional().describe('CEL expression that must evaluate to true on the target record. Enforced at BOTH points in a link\'s life: when the link is minted, and again on every redemption — a record that stops qualifying stops being served through links already minted for it. Fail-closed: a predicate that will not compile, or that faults on the record, refuses.'), }).optional().describe('Public share-link policy (Notion/Figma-style link sharing)'), // [ADR-0085] The former `detail: { … }.passthrough()` UI-hints block is