diff --git a/.changeset/share-link-probe-policy-gate.md b/.changeset/share-link-probe-policy-gate.md new file mode 100644 index 0000000000..a09f1e7910 --- /dev/null +++ b/.changeset/share-link-probe-policy-gate.md @@ -0,0 +1,82 @@ +--- +"@objectstack/plugin-sharing": minor +"@objectstack/runtime": minor +--- + +fix(sharing): gate the share-link route probe on `publicSharing.enabled`, at both probe sites (#14637) + +**BREAKING** runtime behaviour change on a published HTTP path: +`GET /api/v1/share-links/:token/resolve` answers `404 INVALID_OR_EXPIRED` where +it used to answer `401 NEEDS_PASSWORD` / `401 WRONG_PASSWORD` / +`401 SIGN_IN_REQUIRED` / `410 EXPIRED_OR_REVOKED`, for every link whose object +has `publicSharing.enabled` switched off. Shipped as `minor` under the repo's +launch-window convention (a breaking change does not burn a major while the +stack is in lockstep). No published export is added, removed or re-shaped; the +level carries the breaking banner, not a surface change. + +#14033 made `publicSharing.enabled` a standing policy: `resolveToken()` re-reads +the object's current block on every redemption and refuses a switched-off link +with the same undifferentiated `null` a revoked, expired, unknown or ineligible +token gets — because, in that gate's own words, for a caller who may hold +nothing but a token a distinguishable "sharing is off for this object" is an +**existence oracle**. + +The HTTP layer above it then re-opened exactly that oracle. Both share-link +surfaces run a row probe after `resolveToken()` returns null, to answer with a +more useful status, and both answered from the `sys_share_link` row with no +knowledge of the object's block. So an anonymous caller could still tell a +real-but-switched-off token from an unknown one three ways: a row carrying +`password_hash` drew `401 NEEDS_PASSWORD`, the same row with any password drew +`401 WRONG_PASSWORD` — including a **correct** password, which is both an oracle +and a lie, since that link can serve nothing — and a row with +`audience: 'signed_in'` drew `401 SIGN_IN_REQUIRED`. A security property stated +in one layer and defeated in the layer above it is worse than one never claimed, +because the next reader believes the comment. + +**What changed.** Both probes read the object's standing policy before they +answer from the row, and when the block is off every arm falls through to the +generic `404 INVALID_OR_EXPIRED` that unknown, revoked, expired and ineligible +tokens already give — byte-for-byte the answer a token that never existed +receives. The `410 EXPIRED_OR_REVOKED` arm is included: gating only the two 401 +arms would leave a third class of link answer and a rule about which arms are +gated. An object whose schema the engine cannot answer for is `enabled: false` +by `getPolicy`'s definition and is refused the same way — fail-closed, the same +definition `createLink` and `resolveToken` already use. + +The fix lands at **both** sites in one change, because the probe exists twice: +`plugin-sharing`'s REST routes, and the `/share-links` dispatcher domain in +`@objectstack/runtime` that is the designed primary surface for cloud's +per-environment kernels (`registerShareLinkRoutes: false`). Fixing one would +have moved the oracle to whichever embedding uses the other. + +**Nothing else moves.** With the block ON, every refusal is exactly what it was: +`NEEDS_PASSWORD`, `WRONG_PASSWORD`, `SIGN_IN_REQUIRED` and `EXPIRED_OR_REVOKED` +are unchanged in status, code and message, and a correct password or a signed-in +viewer still resolves the record. Mint-time behaviour is untouched, no +`sys_share_link` row is written or read differently, and no error code is added +or retired. + +**Consumer impact.** A viewer that branches on the refusal STATUS sees TWO +changes, for links on a switched-off object only — and the measured consumer +branches on status alone. On the objectui console at `67dadd6`, +`apps/console/src/pages/SharedRecordPage.tsx` lines 70-85 dispatch on +`res.status` and never on the body's error code, so: + +- all three 401 arms (`NEEDS_PASSWORD`, `WRONG_PASSWORD`, `SIGN_IN_REQUIRED`) + rendered the password prompt and now render the 404 copy, "This link is + invalid or no longer available."; +- the 410 arm rendered "This link has expired or was revoked." and now renders + that same 404 copy. + +Both shifts are the intended outcome and were accepted with the ruling: a +correct password on such a link yields nothing, so prompting for one teaches the +holder to open a door that is bricked up, and "expired or revoked" is a claim +about a token whose existence the caller must not be able to confirm. Links on +objects whose block is on are unaffected — prompt, 410 copy and 200 render +included. + +Maintainer ruling 2026-09-03 (decision batch #17, item 1), verbatim 「同意」, +adopting option A over option B (keep the 401 and document the accepted oracle) +and option C (gate only the two 401 arms, rejected as proliferation). + + diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 0e6a08379c..48888df221 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -135,7 +135,7 @@ The largest single consumer — **17 of the 106 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:1476` (guard at `:1501`) | | 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1528` | | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1088` | -| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610` | +| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:469`, `:523`, `:527`, `:600`, `:630` | | 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:165`, `:390` | 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 271ce7c9d5..2d61c76560 100644 --- a/packages/plugins/plugin-sharing/src/share-link-eligibility.test.ts +++ b/packages/plugins/plugin-sharing/src/share-link-eligibility.test.ts @@ -1000,11 +1000,19 @@ describe('[#14033] publicSharing.enabled is a standing policy — the switch is // Before the switch: the link serves the record. expect((await resolve(live.token)).status).toBe(200); + // The pre-existing revoked bucket, recorded as measured WHILE THE BLOCK IS + // ON: a DIFFERENT status, and #14033 does not move it. + // + // [#14637] This reading used to be taken AFTER `switchOff` — see the note + // at the foot of this case for why it moved and what replaced it there. + const revokedWhileOn = await resolve(revoked.token); + expect(revokedWhileOn.status).toBe(410); + expect(revokedWhileOn.body?.error?.code).toBe('EXPIRED_OR_REVOKED'); + switchOff(schemas); const switchedOff = await resolve(live.token); const unknown = await resolve('zzzzzzzzzzzzzzzzzzzzzz'); - const revokedAnswer = await resolve(revoked.token); expect(switchedOff).toEqual(unknown); expect(switchedOff.status).toBe(404); @@ -1015,10 +1023,20 @@ describe('[#14033] publicSharing.enabled is a standing policy — the switch is expect(wire).not.toContain('publicsharing'); expect(wire).not.toContain('sharing_not_enabled'); - // The pre-existing revoked bucket, recorded as measured: a DIFFERENT - // status, and this change does not move it. - expect(revokedAnswer.status).toBe(410); - expect(revokedAnswer.body?.error?.code).toBe('EXPIRED_OR_REVOKED'); + // [#14637] With the block OFF, the 410 arm falls through as well. + // + // This assertion read `410` and was measured AFTER the switch, so what it + // actually pinned was the ROUTE probe answering from the token ROW with no + // knowledge of the object's block — the existence oracle the maintainer + // ruled closed on 2026-09-03 (decision batch #17 item 1, 「同意」 — option + // A: EVERY arm falls through; gating only the two 401 arms was option C and + // was rejected as proliferation). The reading it recorded is not lost: it + // is taken above, while the block is still on, which is where "the revoked + // bucket is a different status and #14033 does not move it" is true. + const revokedWhileOff = await resolve(revoked.token); + expect(revokedWhileOff).toEqual(unknown); + expect(revokedWhileOff.status).toBe(404); + expect(revokedWhileOff.body?.error?.code).toBe('INVALID_OR_EXPIRED'); }); it('the reason a switched-off link died is written to the server-side log, and only there', async () => { @@ -1092,6 +1110,201 @@ describe('[#14033] publicSharing.enabled is a standing policy — the switch is }); }); +/** + * [#14637] The ROUTE probe above `resolveToken`, gated on the SAME standing + * policy — the half the block above could not see. + * + * ## The defect + * + * `resolveToken` refuses a link on a switched-off object with the + * undifferentiated `null` above, and says why in prose: *for a caller who may + * hold nothing but a token, a distinguishable "sharing is off for this object" + * is an existence oracle.* The route then re-opened exactly that oracle one + * layer up. Its probe answered from the ROW — with no knowledge of the + * object's block — so a row carrying `password_hash` still drew + * `401 NEEDS_PASSWORD` / `WRONG_PASSWORD`, and one with + * `audience: 'signed_in'` still drew `401 SIGN_IN_REQUIRED`. A security + * property stated in one layer and defeated in the layer above it is worse + * than one never claimed, because the next reader believes the comment — and + * the sharpest edge of it is that a CORRECT password on a switched-off link + * answered `WRONG_PASSWORD`. + * + * ## What the ruling pins (2026-09-03, decision batch #17 item 1, 「同意」 — A) + * + * Both shapes, on both surfaces, asserting **byte-equality with the + * unknown-token answer** rather than merely "a 404": the claim is that an + * anonymous holder cannot tell the two apart, and only equality of the whole + * captured answer says that. `JSON.stringify` equality is asserted beside + * `toEqual` deliberately — key ORDER is part of what goes on the wire, and a + * deep-equal check does not read it. + * + * The 410 arm is gated too. Gating only the two 401s was option C and was + * rejected as proliferation, so the revoked-link case below is a pin on the + * ruling's shape and not an incidental consequence: on a switched-off object + * even a revoked token stops being distinguishable. + * + * The reverse check is on every case: with the block ON, each 401 (and the + * 410) is exactly what it was. Without it these pins would also pass against a + * route that answered 404 unconditionally, which is not the ruled behaviour. + * + * The dispatcher twin of this probe — the DESIGNED PRIMARY surface for cloud's + * per-environment kernels — is pinned the same way, in + * `runtime/src/domains/share-links-enforcement-context.test.ts`. Landing at one + * site only moves the oracle. + */ +describe('[#14637] the route probe reads the standing policy before it answers from the row', () => { + /** + * `ARTICLE` plus `signed_in` on the audience whitelist — without it the + * `audience: 'signed_in'` shape cannot be MINTED at all + * (`AUDIENCE_NOT_ALLOWED`), so the arm under test would be unreachable. + */ + const SHAREABLE = { + ...ARTICLE, + publicSharing: { ...ARTICLE.publicSharing, allowedAudiences: ['public', 'link_only', 'signed_in'] }, + } as any; + + /** The switch, thrown from outside the token's life. */ + const switchOff = (schemas: Record) => { + schemas.article = { ...SHAREABLE, publicSharing: { ...SHAREABLE.publicSharing, enabled: false } }; + }; + + /** A token that never existed — the answer every gated arm must become. */ + const UNKNOWN_TOKEN = 'zzzzzzzzzzzzzzzzzzzzzz'; + + /** + * The ruling's assertion, in one place: not "a 404" but the SAME answer, + * body and status, that a token which never existed gets. + */ + function expectIndistinguishable(actual: { status: number; body: any }, unknown: { status: number; body: any }) { + expect(actual).toEqual(unknown); + // Byte-equality on the wire, key order included. + expect(JSON.stringify(actual)).toBe(JSON.stringify(unknown)); + expect(actual.status).toBe(404); + expect(actual.body?.error?.code).toBe('INVALID_OR_EXPIRED'); + // Nothing about the policy or the switch reaches the caller. + const wire = JSON.stringify(actual.body).toLowerCase(); + expect(wire).not.toContain('publicsharing'); + expect(wire).not.toContain('sharing_not_enabled'); + expect(wire).not.toContain('password'); + } + + it('the `password_hash` shape — NEEDS_PASSWORD and WRONG_PASSWORD both become the unknown-token answer', async () => { + const { service, engine, schemas } = await boot(SHAREABLE); + const link = await service.createLink( + { object: 'article', recordId: 'a_ok', audience: 'public', permission: 'view', password: 'hunter2' }, + CALLER, + ); + const resolve = mountResolveRoute(service, engine); + + // Reverse check, on the same token: with the block ON the 401 affordance + // is exactly what it always was, in both of its arms. + const needsOn = await resolve(link.token); + expect(needsOn.status).toBe(401); + expect(needsOn.body?.error?.code).toBe('NEEDS_PASSWORD'); + const wrongOn = await resolve(link.token, { password: 'not-it' }); + expect(wrongOn.status).toBe(401); + expect(wrongOn.body?.error?.code).toBe('WRONG_PASSWORD'); + // …and the correct password serves the record. + expect((await resolve(link.token, { password: 'hunter2' })).status).toBe(200); + + switchOff(schemas); + const unknown = await resolve(UNKNOWN_TOKEN); + + expectIndistinguishable(await resolve(link.token), unknown); + expectIndistinguishable(await resolve(link.token, { password: 'not-it' }), unknown); + // THE SHARPEST EDGE: a CORRECT password on a switched-off link used to + // answer `WRONG_PASSWORD`, which is both an oracle and a lie. + expectIndistinguishable(await resolve(link.token, { password: 'hunter2' }), unknown); + }); + + it("the `audience: 'signed_in'` shape — SIGN_IN_REQUIRED becomes the unknown-token answer", async () => { + const { service, engine, schemas } = await boot(SHAREABLE); + const link = await service.createLink( + { object: 'article', recordId: 'a_ok', audience: 'signed_in', permission: 'view' }, + CALLER, + ); + const anonymous = mountResolveRoute(service, engine); + const signedIn = mountResolveRoute(service, engine, 'u1'); + + // Reverse check: with the block ON an anonymous caller is still told to + // sign in, and a signed-in one is still served. + const on = await anonymous(link.token); + expect(on.status).toBe(401); + expect(on.body?.error?.code).toBe('SIGN_IN_REQUIRED'); + expect((await signedIn(link.token)).status).toBe(200); + + switchOff(schemas); + expectIndistinguishable(await anonymous(link.token), await anonymous(UNKNOWN_TOKEN)); + // The signed-in caller loses the record too — the switch is a standing + // policy, not an authentication affordance. + expectIndistinguishable(await signedIn(link.token), await signedIn(UNKNOWN_TOKEN)); + }); + + it('EVERY arm falls through, the 410 included — option C (gate only the two 401s) is not what shipped', async () => { + const { service, engine, schemas } = await boot(SHAREABLE); + const link = await service.createLink( + { object: 'article', recordId: 'a_ok', audience: 'public', permission: 'view' }, + CALLER, + ); + await service.revokeLink(link.token, { isSystem: true } as any); + const resolve = mountResolveRoute(service, engine); + + // Reverse check: with the block ON, the revoked bucket is untouched — this + // is the pre-existing behaviour #14033 measured and did not move. + const on = await resolve(link.token); + expect(on.status).toBe(410); + expect(on.body?.error?.code).toBe('EXPIRED_OR_REVOKED'); + + switchOff(schemas); + expectIndistinguishable(await resolve(link.token), await resolve(UNKNOWN_TOKEN)); + }); + + it("the 410 arm's OTHER half — an EXPIRED link falls through too, not just a revoked one", async () => { + const { driver, service, engine, schemas } = await boot(SHAREABLE); + const link = await service.createLink( + { object: 'article', recordId: 'a_ok', audience: 'public', permission: 'view' }, + CALLER, + ); + // Back-dated on the stored row, not minted: `createLink` refuses a past + // `expiresAt` outright (`422 EXPIRY_IN_PAST`), so this is the only way to + // reach an ALREADY-EXPIRED link — and it is exactly what the passage of + // time does to a live one. Same stamp the #13608 pins above use. + await driver.update('sys_share_link', link.id, { + expires_at: new Date(Date.now() - 60_000).toISOString(), + }); + const resolve = mountResolveRoute(service, engine); + + // `revoked_at` and `expires_at` are two predicates reaching ONE arm, so a + // pin on the revoked half alone leaves the expired half free to keep + // answering 410 on a switched-off object — the same oracle, reached by the + // other predicate. + // + // Reverse check: with the block ON, an expired link is still 410. + const on = await resolve(link.token); + expect(on.status).toBe(410); + expect(on.body?.error?.code).toBe('EXPIRED_OR_REVOKED'); + + switchOff(schemas); + expectIndistinguishable(await resolve(link.token), await resolve(UNKNOWN_TOKEN)); + }); + + it('fail-closed: an object whose schema the engine cannot answer for is refused, not probed', async () => { + const { service, engine, schemas } = await boot(SHAREABLE); + const link = await service.createLink( + { object: 'article', recordId: 'a_ok', audience: 'public', permission: 'view', password: 'hunter2' }, + CALLER, + ); + const resolve = mountResolveRoute(service, engine); + + // Not "off" — GONE. `getPolicy` calls an unanswerable schema + // `enabled: false`, and the route reaches the same verdict rather than + // falling back to the row. + delete schemas.article; + + expectIndistinguishable(await resolve(link.token, { password: 'hunter2' }), await resolve(UNKNOWN_TOKEN)); + }); +}); + /** * [#13608] Mount the real PUBLIC resolve route on the real service. * @@ -1099,8 +1312,15 @@ describe('[#14033] publicSharing.enabled is a standing policy — the switch is * 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. + * + * [#14637] Two optional widenings, both inert for every caller that predates + * them: `signedInUserId` supplies a VERIFIED identity (the only way to reach + * the `audience: 'signed_in'` arm from the serving side), and the returned + * driver takes the request query (the only way to reach the `WRONG_PASSWORD` + * arm). Omit both and this is byte-for-byte the anonymous, query-less harness + * the #13608 and #14033 pins above drive. */ -function mountResolveRoute(service: ShareLinkService, engine: unknown) { +function mountResolveRoute(service: ShareLinkService, engine: unknown, signedInUserId?: string) { const routes = new Map(); const http: any = { get: (path: string, h: RouteHandler) => { routes.set(`GET ${path}`, h); return http; }, @@ -1113,12 +1333,20 @@ function mountResolveRoute(service: ShareLinkService, engine: unknown) { close: async () => undefined, getInstance: () => null, }; - registerShareLinkRoutes(http as IHttpServer, service, engine as any); + registerShareLinkRoutes( + http as IHttpServer, + service, + engine as any, + signedInUserId ? { contextFromRequest: () => ({ userId: signedInUserId }) } : {}, + ); 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 }> => { + return async ( + token: string, + query: Record = {}, + ): 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; }, @@ -1126,7 +1354,7 @@ function mountResolveRoute(service: ShareLinkService, engine: unknown) { send: () => res, header: () => res, }; - const req: any = { params: { token }, query: {}, headers: {}, method: 'GET', path: '/' }; + 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-envelope.conformance.test.ts b/packages/plugins/plugin-sharing/src/share-link-envelope.conformance.test.ts index 5f56ff8df4..215b74607c 100644 --- a/packages/plugins/plugin-sharing/src/share-link-envelope.conformance.test.ts +++ b/packages/plugins/plugin-sharing/src/share-link-envelope.conformance.test.ts @@ -90,6 +90,26 @@ function mount(overrides: { const engine = { find: vi.fn(async () => [{ id: 'acc_1', name: 'Acme', ssn: '123-45-6789' }]), + // [#14637] The probe above the row arms reads the object's STANDING + // `publicSharing` policy before it answers from the row, so an engine that + // cannot answer `getSchema` refuses every arm with the generic 404 — + // correctly, and fail-closed. This double therefore has to DECLARE the + // block for the object its probe rows name, or the 401 / 410 refusals the + // envelope cases below drive are simply not reachable. That is a fixture + // declaration, not a relaxation: the gated behaviour itself is pinned in + // `share-link-eligibility.test.ts`, and this module's subject is the + // ENVELOPE each refusal is written in. + getSchema: vi.fn((name: string) => + name === 'crm_account' + ? { + name, + publicSharing: { + enabled: true, + allowedAudiences: ['link_only', 'signed_in'], + allowedPermissions: ['view'], + }, + } + : undefined), insert: vi.fn(), update: vi.fn(), delete: vi.fn(), ...(overrides.engine ?? {}), } as unknown as SharingEngine; @@ -337,7 +357,7 @@ describe('share-link envelope (#3983) — error bodies', () => { run: async () => drive( mount({ service: { resolveToken: vi.fn(async () => null) }, - engine: { find: vi.fn(async () => [{ token: 'tok_abcdefgh', password_hash: 'h', revoked_at: null, expires_at: null }]) }, + engine: { find: vi.fn(async () => [{ token: 'tok_abcdefgh', object_name: 'crm_account', password_hash: 'h', revoked_at: null, expires_at: null }]) }, }).http, `GET ${B}/:token/resolve`, { params: { token: 'tok_abcdefgh' } }, @@ -350,7 +370,7 @@ describe('share-link envelope (#3983) — error bodies', () => { run: async () => drive( mount({ service: { resolveToken: vi.fn(async () => null) }, - engine: { find: vi.fn(async () => [{ token: 'tok_abcdefgh', password_hash: 'h', revoked_at: null, expires_at: null }]) }, + engine: { find: vi.fn(async () => [{ token: 'tok_abcdefgh', object_name: 'crm_account', password_hash: 'h', revoked_at: null, expires_at: null }]) }, }).http, `GET ${B}/:token/resolve`, { params: { token: 'tok_abcdefgh' }, query: { password: 'nope' } }, @@ -364,7 +384,7 @@ describe('share-link envelope (#3983) — error bodies', () => { mount({ userId: undefined, service: { resolveToken: vi.fn(async () => null) }, - engine: { find: vi.fn(async () => [{ token: 'tok_abcdefgh', audience: 'signed_in', revoked_at: null, expires_at: null }]) }, + engine: { find: vi.fn(async () => [{ token: 'tok_abcdefgh', object_name: 'crm_account', audience: 'signed_in', revoked_at: null, expires_at: null }]) }, }).http, `GET ${B}/:token/resolve`, { params: { token: 'tok_abcdefgh' } }, @@ -377,7 +397,7 @@ describe('share-link envelope (#3983) — error bodies', () => { run: async () => drive( mount({ service: { resolveToken: vi.fn(async () => null) }, - engine: { find: vi.fn(async () => [{ token: 'tok_abcdefgh', revoked_at: '2026-01-02T00:00:00.000Z' }]) }, + engine: { find: vi.fn(async () => [{ token: 'tok_abcdefgh', object_name: 'crm_account', revoked_at: '2026-01-02T00:00:00.000Z' }]) }, }).http, `GET ${B}/:token/resolve`, { params: { token: 'tok_abcdefgh' } }, diff --git a/packages/plugins/plugin-sharing/src/share-link-routes.ts b/packages/plugins/plugin-sharing/src/share-link-routes.ts index 28ce112f42..03632ed773 100644 --- a/packages/plugins/plugin-sharing/src/share-link-routes.ts +++ b/packages/plugins/plugin-sharing/src/share-link-routes.ts @@ -35,7 +35,11 @@ import type { IHttpServer, IHttpRequest, RouteHandler } from '@objectstack/spec/ import { sendOk, sendError } from '@objectstack/types'; import type { ShareLinkExecutionContext } from '@objectstack/spec/contracts'; import type { ExecutionContext } from '@objectstack/spec/kernel'; -import type { ShareLinkService } from './share-link-service.js'; +// [#14637] `isPublicSharingEnabled` is the service's OWN reading of the +// standing switch, imported rather than restated here. A second spelling of +// `publicSharing.enabled` at this layer is how the probe below came to +// contradict the gate inside `resolveToken` in the first place. +import { isPublicSharingEnabled, type ShareLinkService } from './share-link-service.js'; import type { SharingEngine } from './sharing-service.js'; const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; @@ -236,6 +240,12 @@ export function registerShareLinkRoutes( providedPassword, }); if (!resolved) { + // The ONE generic refusal this route answers with. Written once so the + // arms that fall through to it are byte-identical to it by + // construction rather than by three copies staying in step (#14637). + const invalidOrExpired = () => + sendError(res, 404, 'INVALID_OR_EXPIRED', 'Share link is invalid, expired, or revoked'); + // Probe row to give a more useful status code (401 vs 404 vs 410). const probe = await engine.find('sys_share_link', { where: { token: req.params.token }, @@ -243,6 +253,38 @@ export function registerShareLinkRoutes( context: SYSTEM_CTX, } as any); const row = Array.isArray(probe) && probe[0] ? (probe[0] as any) : null; + // [#14637 — maintainer ruling 2026-09-03, decision batch #17 item 1, + // verbatim 「同意」 — option A] The standing policy is read BEFORE this + // probe answers from the row. + // + // `resolveToken` refuses a link whose object has `publicSharing.enabled` + // off, and refuses it with the undifferentiated `null` that revoked / + // expired / unknown / ineligible tokens get, because — in its own words + // — for a caller who may hold nothing but a token, a distinguishable + // "sharing is off for this object" is an existence oracle. This probe + // then re-opened exactly that oracle one layer up: it answered from the + // ROW, with no knowledge of the object's block, so a `password_hash` + // row still drew `401 NEEDS_PASSWORD` / `WRONG_PASSWORD` and an + // `audience: 'signed_in'` row still drew `401 SIGN_IN_REQUIRED` — a + // real-but-switched-off token, told apart from an unknown one by an + // anonymous caller. A security property stated in one layer and + // defeated in the layer above it is worse than one never claimed, + // because the next reader believes the comment. + // + // EVERY arm falls through, the 410 included: gating only the two 401s + // was option C and was rejected as proliferation — it would leave a + // third class of link answer and a rule about which arms are gated. + // The accepted product consequence: on a switched-off object the viewer + // sees "link invalid" rather than a password prompt, which is correct, + // since a correct password on such a link yields nothing. + // + // Fail-closed on an unreadable policy is deliberate and costs nothing + // real: `getPolicy` calls a schema the engine cannot return + // `enabled: false` by the same definition, so `resolveToken` on this + // same engine has already refused every token on that object. + if (row && !isPublicSharingEnabled(engine.getSchema?.(row.object_name))) { + return invalidOrExpired(); + } if (row && !row.revoked_at && (!row.expires_at || Date.parse(row.expires_at) > Date.now())) { if (row.password_hash) { return sendError( @@ -259,7 +301,7 @@ export function registerShareLinkRoutes( if (row && (row.revoked_at || (row.expires_at && Date.parse(row.expires_at) <= Date.now()))) { return sendError(res, 410, 'EXPIRED_OR_REVOKED', 'Share link has expired or been revoked'); } - return sendError(res, 404, 'INVALID_OR_EXPIRED', 'Share link is invalid, expired, or revoked'); + return invalidOrExpired(); } // Fetch the underlying record with system context — the token diff --git a/packages/plugins/plugin-sharing/src/share-link-service.ts b/packages/plugins/plugin-sharing/src/share-link-service.ts index 3d5215c37f..6fbe885c6f 100644 --- a/packages/plugins/plugin-sharing/src/share-link-service.ts +++ b/packages/plugins/plugin-sharing/src/share-link-service.ts @@ -85,6 +85,26 @@ function generateToken(length: number = TOKEN_LENGTH): string { return out; } +/** + * [#14637] Is `publicSharing` switched ON for this object schema? + * + * The ONE reading of the standing switch, exported so the HTTP probe that sits + * ABOVE `resolveToken` asks the same question the gate INSIDE it asks. It was + * a private expression here while the route layer answered from the token row + * with no knowledge of the object's block, which re-opened the existence + * oracle this service's redemption gate closes (maintainer ruling 2026-09-03, + * decision batch #17 item 1, verbatim 「同意」 — option A). + * + * An absent block, an absent schema, and an engine that cannot answer + * `getSchema` at all are one answer: `false`. `enabled` defaults to off, so a + * caller that cannot read the policy must refuse rather than answer from the + * row — the same definition {@link getPolicy} has always used. + */ +export function isPublicSharingEnabled(schema: unknown): boolean { + return (schema as { publicSharing?: { enabled?: unknown } } | null | undefined) + ?.publicSharing?.enabled === true; +} + /** Internal helper — extract publicSharing policy from an object schema. */ function getPolicy(schema: any): { enabled: boolean; @@ -95,7 +115,7 @@ function getPolicy(schema: any): { eligibility?: string; } { const raw = schema?.publicSharing; - if (!raw || raw.enabled !== true) { + if (!isPublicSharingEnabled(schema)) { return { enabled: false, allowedAudiences: [], diff --git a/packages/runtime/src/domains/share-links-enforcement-context.test.ts b/packages/runtime/src/domains/share-links-enforcement-context.test.ts index a01993b17b..cf0e37e4b1 100644 --- a/packages/runtime/src/domains/share-links-enforcement-context.test.ts +++ b/packages/runtime/src/domains/share-links-enforcement-context.test.ts @@ -164,7 +164,7 @@ function matches(row: any, filter: any): boolean { * AND-composes Layer 0 + Layer 1 into) and executes the COMPOSED predicate — * so a deny verdict here is the production middleware's, not this file's. */ -function makeEngine(tables: Record) { +function makeEngine(tables: Record, schemas?: Record) { const middlewares: Array<(opCtx: any, next: () => Promise) => Promise> = []; const runChain = async (opCtx: any, terminal: () => Promise): Promise => { const dispatch = async (i: number): Promise => @@ -174,7 +174,13 @@ function makeEngine(tables: Record) { return { _tables: tables, registerMiddleware: (mw: any) => middlewares.push(mw), - getSchema: (name: string) => (name === OBJECT ? ACCOUNT_SCHEMA : { name }), + // [#14637] With a `schemas` map the double answers from IT and from + // nothing else — including `undefined` for a name it does not carry, + // which is the engine-cannot-answer case the policy gate must fail + // CLOSED on. Omit the map (every caller that predates #14637) and this + // is byte-for-byte the previous behaviour. + getSchema: (name: string) => + schemas ? schemas[name] : name === OBJECT ? ACCOUNT_SCHEMA : { name }, async find(object: string, options: any = {}) { const opCtx: any = { object, @@ -672,3 +678,242 @@ describe('[#6649] a security-middleware refusal keeps its own status through the expect(expectDeclaredEnvelope(res).code).toBe('SHARING_NOT_ENABLED'); }); }); + +/** + * [#14637] The DISPATCHER twin of the share-link route probe, gated on the + * standing `publicSharing.enabled` policy. + * + * ## Why this pin lives here and not only in plugin-sharing + * + * The probe above `resolveToken` exists twice: once in + * `plugin-sharing/src/share-link-routes.ts` and once in the domain under test + * here. For cloud's per-environment kernels this one is the DESIGNED PRIMARY + * surface (`registerShareLinkRoutes: false`, see this module's header), so a + * fix landing only at the plugin site would not close the oracle — it would + * move it to whichever embedding uses this one. That is why the maintainer's + * ruling (2026-09-03, decision batch #17 item 1, verbatim 「同意」 — option A) + * names both sites in one PR, and why both are pinned. + * + * ## What was wrong + * + * `resolveToken` refuses a link whose object has `publicSharing.enabled` off, + * and refuses it with the undifferentiated `null` that revoked / expired / + * unknown / ineligible tokens get, because a distinguishable "sharing is off + * for this object" is an existence oracle for a caller who may hold nothing + * but a token. This probe then answered from the ROW: a `password_hash` row + * drew `401 NEEDS_PASSWORD` / `WRONG_PASSWORD`, an `audience: 'signed_in'` row + * drew `401 SIGN_IN_REQUIRED`, and a revoked row on a switched-off object drew + * `410 EXPIRED_OR_REVOKED` — three ways to tell a real-but-switched-off token + * from an unknown one. + * + * ## What is asserted + * + * Byte-equality with the unknown-token answer, not merely "a 404": the claim is + * that an anonymous holder cannot tell them apart, and only equality of the + * whole answer says that. `JSON.stringify` equality sits beside `toEqual` + * because key ORDER is part of what goes on the wire. + * + * Every case carries its reverse check — with the block ON the 401s and the + * 410 are exactly what they were. Without it these pins would also pass + * against a probe that answered 404 unconditionally, which is not the ruled + * behaviour. + * + * REAL here: the domain body, `ShareLinkService`, and the ADR-0112 envelope + * builder. DOUBLE: storage (`makeEngine` above, whose write verbs open with the + * producers' own dispatch predicates) — deliberately WITHOUT `bootSecurity`, + * since no verdict below depends on the middleware chain and the resolve route + * reads under `SYSTEM_CTX` anyway. + */ +describe('[#14637] the dispatcher probe reads the standing policy before it answers from the row', () => { + const SHARED_OBJECT = 'kb_article'; + const SHARED_RECORD = 'kb_1'; + const UNKNOWN_TOKEN = 'zzzzzzzzzzzzzzzzzzzzzz'; + + /** The shareable object, with `signed_in` on the whitelist so that arm is mintable. */ + const shareableSchema = (enabled: boolean) => ({ + name: SHARED_OBJECT, + fields: { + id: { name: 'id' }, + title: { name: 'title' }, + owner_id: { name: 'owner_id' }, + }, + publicSharing: { + enabled, + allowedAudiences: ['link_only', 'signed_in'], + allowedPermissions: ['view'], + }, + }); + + interface Harness { + /** Drive `GET /share-links/:token/resolve` on the production domain body. */ + resolve(token: string, opts?: { password?: string; signedIn?: boolean }): Promise<{ status: number; body: any }>; + /** Throw the object's switch, from outside the token's life. */ + switchOff(): void; + /** Take the object's schema away entirely — the unanswerable-policy case. */ + forgetSchema(): void; + mint(input: { audience?: 'link_only' | 'signed_in'; password?: string }): Promise; + revoke(token: string): Promise; + /** Let the token's own clock run out — the OTHER half of the same 410 arm. */ + expire(token: string): void; + } + + async function harness(): Promise { + const schemas: Record = { [SHARED_OBJECT]: shareableSchema(true) }; + const tables: Record = { + [SHARED_OBJECT]: [{ id: SHARED_RECORD, title: 'How to share', owner_id: USER }], + sys_share_link: [], + }; + const engine = makeEngine(tables, schemas); + const svc = new ShareLinkService({ engine: engine as any }); + const deps = makeDeps(engine, svc); + + return { + switchOff: () => { schemas[SHARED_OBJECT] = shareableSchema(false); }, + forgetSchema: () => { delete schemas[SHARED_OBJECT]; }, + mint: async ({ audience = 'link_only', password }) => { + const link = await svc.createLink( + { + object: SHARED_OBJECT, + recordId: SHARED_RECORD, + audience, + permission: 'view', + ...(password ? { password } : {}), + }, + envelopeFor({ memberOf: [ORG_A], permissions: ['acct_member'] }), + ); + expect(link.token).toBeTruthy(); + return link.token; + }, + revoke: async (token: string) => { await svc.revokeLink(token, { isSystem: true } as any); }, + // Stamped on the row rather than minted: `createLink` refuses a past + // `expiresAt` outright (`422 EXPIRY_IN_PAST`), so back-dating the + // stored row is the only way to reach an ALREADY-EXPIRED link — which + // is exactly what the passage of time does to a live one. + expire: (token: string) => { + const row = (tables.sys_share_link ?? []).find((r) => r.token === token); + if (!row) throw new Error('expire(): no sys_share_link row for that token'); + row.expires_at = new Date(Date.now() - 60_000).toISOString(); + }, + resolve: async (token, opts = {}) => { + const res = await handleShareLinksRequest( + deps, + `/${token}/resolve`, + 'GET', + undefined, + opts.password ? { password: opts.password } : {}, + httpContext( + opts.signedIn + ? envelopeFor({ memberOf: [ORG_A], permissions: ['acct_member'] }) + : undefined, + ), + ); + if (!res.handled || !res.response) throw new Error('GET /share-links/:token/resolve was not handled'); + return res.response as { status: number; body: any }; + }, + }; + } + + /** The ruling's assertion: not "a 404" but the SAME answer a never-minted token gets. */ + function expectIndistinguishable(actual: { status: number; body: any }, unknown: { status: number; body: any }) { + expect(actual).toEqual(unknown); + // Byte-equality on the wire, key order included. + expect(JSON.stringify(actual)).toBe(JSON.stringify(unknown)); + expect(actual.status).toBe(404); + expect(actual.body?.error?.code).toBe('INVALID_OR_EXPIRED'); + const wire = JSON.stringify(actual.body).toLowerCase(); + expect(wire).not.toContain('publicsharing'); + expect(wire).not.toContain('sharing_not_enabled'); + expect(wire).not.toContain('password'); + } + + it('the `password_hash` shape — NEEDS_PASSWORD and WRONG_PASSWORD both become the unknown-token answer', async () => { + const h = await harness(); + const token = await h.mint({ password: 'hunter2' }); + + // Reverse check, on the same token: with the block ON the 401 + // affordance is exactly what it always was, in both arms. + const needsOn = await h.resolve(token); + expect(needsOn.status).toBe(401); + expect(needsOn.body?.error?.code).toBe('NEEDS_PASSWORD'); + const wrongOn = await h.resolve(token, { password: 'not-it' }); + expect(wrongOn.status).toBe(401); + expect(wrongOn.body?.error?.code).toBe('WRONG_PASSWORD'); + expect((await h.resolve(token, { password: 'hunter2' })).status).toBe(200); + + h.switchOff(); + const unknown = await h.resolve(UNKNOWN_TOKEN); + + expectIndistinguishable(await h.resolve(token), unknown); + expectIndistinguishable(await h.resolve(token, { password: 'not-it' }), unknown); + // THE SHARPEST EDGE: a CORRECT password on a switched-off link answered + // `WRONG_PASSWORD` — both an oracle and a lie. + expectIndistinguishable(await h.resolve(token, { password: 'hunter2' }), unknown); + }); + + it("the `audience: 'signed_in'` shape — SIGN_IN_REQUIRED becomes the unknown-token answer", async () => { + const h = await harness(); + const token = await h.mint({ audience: 'signed_in' }); + + // Reverse check: an anonymous caller is still told to sign in, a + // signed-in one is still served. + const on = await h.resolve(token); + expect(on.status).toBe(401); + expect(on.body?.error?.code).toBe('SIGN_IN_REQUIRED'); + expect((await h.resolve(token, { signedIn: true })).status).toBe(200); + + h.switchOff(); + expectIndistinguishable(await h.resolve(token), await h.resolve(UNKNOWN_TOKEN)); + // The signed-in caller loses it too — a standing policy, not an + // authentication affordance. + expectIndistinguishable( + await h.resolve(token, { signedIn: true }), + await h.resolve(UNKNOWN_TOKEN, { signedIn: true }), + ); + }); + + it('EVERY arm falls through, the 410 included — option C (gate only the two 401s) is not what shipped', async () => { + const h = await harness(); + const token = await h.mint({}); + await h.revoke(token); + + // Reverse check: with the block ON the revoked bucket is untouched. + const on = await h.resolve(token); + expect(on.status).toBe(410); + expect(on.body?.error?.code).toBe('EXPIRED_OR_REVOKED'); + + h.switchOff(); + expectIndistinguishable(await h.resolve(token), await h.resolve(UNKNOWN_TOKEN)); + }); + + it("the 410 arm's OTHER half — an EXPIRED link falls through too, not just a revoked one", async () => { + const h = await harness(); + const token = await h.mint({}); + h.expire(token); + + // `revoked_at` and `expires_at` are two predicates reaching ONE arm + // (`share-links.ts`: `row.revoked_at || (row.expires_at && …)`), so a + // pin on the revoked half alone leaves the expired half free to keep + // answering 410 on a switched-off object — the same oracle, reached by + // the other predicate. + // + // Reverse check: with the block ON, an expired link is still 410. + const on = await h.resolve(token); + expect(on.status).toBe(410); + expect(on.body?.error?.code).toBe('EXPIRED_OR_REVOKED'); + + h.switchOff(); + expectIndistinguishable(await h.resolve(token), await h.resolve(UNKNOWN_TOKEN)); + }); + + it('fail-closed: an object whose schema the engine cannot answer for is refused, not probed', async () => { + const h = await harness(); + const token = await h.mint({ password: 'hunter2' }); + + // Not "off" — GONE. `getPolicy` calls an unanswerable schema + // `enabled: false`, and this probe reaches the same verdict rather than + // falling back to the row. + h.forgetSchema(); + + expectIndistinguishable(await h.resolve(token, { password: 'hunter2' }), await h.resolve(UNKNOWN_TOKEN)); + }); +}); diff --git a/packages/runtime/src/domains/share-links.ts b/packages/runtime/src/domains/share-links.ts index 55ee86fa8d..6b14a2e197 100644 --- a/packages/runtime/src/domains/share-links.ts +++ b/packages/runtime/src/domains/share-links.ts @@ -40,6 +40,29 @@ import { SHARE_LINK_SERVICE } from '@objectstack/spec/contracts'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; +/** + * [#14637] Is `publicSharing` switched ON for this object schema? + * + * A deliberate MIRROR of `isPublicSharingEnabled` in + * `plugin-sharing/src/share-link-service.ts`, which is the canonical + * definition and the one `resolveToken`'s own gate reads. It is copied rather + * than imported because `@objectstack/plugin-sharing` is a **dev** dependency + * of this package: importing it here would invert the dependency direction to + * make one boolean read shared. The two spellings are held equal by the pins + * in `share-links-enforcement-context.test.ts` on this side and + * `share-link-eligibility.test.ts` on the other, which assert the SAME + * observable answer on both surfaces rather than trusting the copy. + * + * An absent block, an absent schema, and an engine that cannot answer + * `getSchema` at all are one answer: `false`. `enabled` defaults to off, so a + * surface that cannot read the policy must refuse rather than answer from the + * token row. + */ +function isPublicSharingEnabled(schema: unknown): boolean { + return (schema as { publicSharing?: { enabled?: unknown } } | null | undefined) + ?.publicSharing?.enabled === true; +} + export function createShareLinksDomain(deps: DomainHandlerDeps): DomainRoute { return { prefix: '/share-links', @@ -104,6 +127,11 @@ export async function handleShareLinksRequest( handled: true, response: deps.error(msg, status, { code }), }); + // The ONE generic refusal the resolve route answers with. Written once so + // the arms that fall through to it are byte-identical to it by + // construction rather than by copies staying in step (#14637). + const invalidOrExpired = (): HttpDispatcherResult => + sendErr(404, 'INVALID_OR_EXPIRED', 'Share link is invalid, expired, or revoked'); // Engine for fetching the shared record + token probes — the same // per-env ObjectQL the shareLinks service is bound to. Read from the // request's RESOLVED (per-env) kernel first: `resolveService('objectql', @@ -149,6 +177,37 @@ export async function handleShareLinksRequest( ? asArray(await engine.find('sys_share_link', { where: { token }, limit: 1, context: SYSTEM_CTX } as any)) : []; const row = probe[0] ?? null; + // [#14637 — maintainer ruling 2026-09-03, decision batch #17 + // item 1, verbatim 「同意」 — option A] The standing policy is + // read BEFORE this probe answers from the row. + // + // This is the dispatcher twin of the same probe in + // `plugin-sharing/src/share-link-routes.ts`, and for cloud's + // per-environment kernels it is the DESIGNED PRIMARY surface + // (`registerShareLinkRoutes: false`) — so fixing only the other + // site would not close the oracle, it would move it to + // whichever embedding uses this one. + // + // `resolveToken` refuses a link whose object has + // `publicSharing.enabled` off with the undifferentiated `null` + // revoked / expired / unknown / ineligible tokens get, because + // a distinguishable "sharing is off for this object" is an + // existence oracle for a caller who may hold nothing but a + // token. Answering from the ROW re-opened it: a `password_hash` + // row still drew `401 NEEDS_PASSWORD` / `WRONG_PASSWORD`, an + // `audience: 'signed_in'` row still drew `401 + // SIGN_IN_REQUIRED`. EVERY arm falls through here, the 410 + // included — gating only the two 401s was option C and was + // rejected as proliferation. + // + // Fail-closed on an unreadable policy is deliberate: an object + // whose schema the engine cannot return is `enabled: false` by + // the canonical definition, which is the same answer the + // service's own gate reaches. + if (row) { + const schema = engine?.getSchema(row.object_name); + if (!isPublicSharingEnabled(schema)) return invalidOrExpired(); + } const live = row && !row.revoked_at && (!row.expires_at || Date.parse(row.expires_at) > Date.now()); if (live && row.password_hash) { return sendErr(401, providedPassword ? 'WRONG_PASSWORD' : 'NEEDS_PASSWORD', @@ -160,7 +219,7 @@ export async function handleShareLinksRequest( if (row && (row.revoked_at || (row.expires_at && Date.parse(row.expires_at) <= Date.now()))) { return sendErr(410, 'EXPIRED_OR_REVOKED', 'Share link has expired or been revoked'); } - return sendErr(404, 'INVALID_OR_EXPIRED', 'Share link is invalid, expired, or revoked'); + return invalidOrExpired(); } const engine = await getEngine();