Skip to content

Commit d1e2841

Browse files
fix(plugin-sharing): an email-audience share link's refusal does the same work whatever email and password are presented (#23506)
Fixes #23474 Clause-②: no ## Summary An `email`-audience share link's refusal now does the same work whatever email and password are presented, as its answer bytes already did after PR #23471 (#23372). A reachability measurement on both resolve mounts found the difference observable before this change and within noise after it; the full readings are held by the owning seat under this lane's disclosure rule. ## The change `packages/plugins/plugin-sharing/src/share-link-service.ts`, `resolveToken`: for a live row whose audience is `email`, the audience verdict and the password verdict are combined at the end, so the refusal path is the same whatever the caller presents. The legacy-hash upgrade stays on the fully-passed path only. This is the landing point the card named, the gate both resolve mounts share, so no other package changes. ## Scope - **Answer bytes unchanged.** The refusal and the served response are byte-for-byte what PR #23471 pinned. The refusal-order table (`packages/runtime/src/domains/share-links-refusal-order.test.ts`) passes untouched, 60 of 60. - **Scoped to the `email` audience.** The `signed_in` early return is kept. It reveals only whether the caller is signed in, which the caller already knows, so it says nothing about a server-side secret. No reason to widen the change was measured. - **Not a goal: hiding whether a token exists.** An unknown token stays a fast miss, and tokens are unguessable. ## Tests Local verification at `0088cf19`: - New pin `packages/plugins/plugin-sharing/src/share-link-service.email-refusal-cost.pin.test.ts`. On an `email`+password link it counts the refusal's work per request, for an off-list and an allowlisted email, once with a wrong password and once with none: the counts must be equal. Timing is flaky in CI, the counts are not. A control checks that an allowlisted email with the correct password still serves the record. - The pin was red on the pre-fix `resolveToken` (2 of 3 failed, the control passed) and is green after the fix. - Ablation at `0088cf19`, run through `scripts/ablation-replace.mjs`: putting the pre-fix ordering back turns both pair tests red on unequal work counts; the file is then restored, its blob equals HEAD and `git diff HEAD` is empty. - `@objectstack/plugin-sharing`: `test` 46 files / 1061 tests passed; `typecheck` exit 0, and the new pin is in the test-layer program. - `@objectstack/runtime`: `share-links-refusal-order.test.ts` 60 / 60, against a rebuilt plugin-sharing `dist`. - Gates: the 65 families `dispatch-gates.mjs --commands` derives at `0088cf19`, every one exit 0; `--ran` reconciliation: 65 derived, 65 run, 0 NOT-MEASURED. The full farm is CI's. The branch is based at `87315102`. The four commits `main` has gained since touch none of the paths here. --- _Generated by [Claude Code](https://claude.ai/code/session_01LxHPTb1LxL7a6WkQbALKnD)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3f7cb1b commit d1e2841

3 files changed

Lines changed: 196 additions & 10 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/plugin-sharing': patch
3+
---
4+
5+
fix(plugin-sharing): an email-audience share link's refusal does the same work whatever email and password are presented
6+
7+
The refusal of an `email`-audience share link no longer does a different amount
8+
of work depending on the caller's input, as its answer bytes already did not.
9+
The refusal and the served response are byte-for-byte unchanged.
10+
11+
Clause-②: no
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#23474] An `email`-audience share link's refusal must do the SAME WORK
5+
* whatever email and password are presented.
6+
*
7+
* ## The property this pins
8+
*
9+
* PR #23471 (#23372) made every refusal of an `email`-audience link
10+
* byte-identical on both mounts, so no caller can read allowlist membership
11+
* out of the ANSWER. This pins the other half: the refusal's WORK does not
12+
* depend on the email presented either, since work that differs on whether an
13+
* address is on the list is a channel whatever the answer bytes are.
14+
*
15+
* Timing is flaky in CI, so the work is pinned deterministically rather than
16+
* the clock: for an `email`+password link, the work is counted per request and
17+
* must be equal for an off-list email and an allowlisted one, whether a wrong
18+
* password or no password is presented.
19+
*
20+
* ## What is real here
21+
*
22+
* One real `ObjectQL` on `@objectstack/driver-sql` + better-sqlite3 with the
23+
* shipped `sys_share_link` definition (whose `password_hash` is `internal`, so
24+
* the hash is recovered through `resolveInternalField` — the call this counts).
25+
* The password verifier is the production `verifyShareLinkPassword`, wrapped to
26+
* count. Both resolve mounts call this one `resolveToken`, so the cost lives
27+
* here, where one pin covers both.
28+
*
29+
* The controls stay green: an allowlisted email with the correct password still
30+
* serves the record, and the refusals stay the one generic `null`.
31+
*/
32+
33+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
34+
import { ObjectQL } from '@objectstack/objectql';
35+
import { SqlDriver } from '@objectstack/driver-sql';
36+
import { ShareLinkService } from './share-link-service.js';
37+
import { SysShareLink } from './objects/sys-share-link.object.js';
38+
import { verifyShareLinkPassword } from './share-link-password.js';
39+
40+
const OBJECT = 'refusal_cost_doc';
41+
const RECORD = 'doc_1';
42+
const PASSWORD = 'correct horse 23474';
43+
const WRONG = 'not the password 23474';
44+
const ALLOWLISTED = 'recipient-23474@example.test';
45+
const OFFLIST = 'offlist-23474@example.test';
46+
47+
const TARGET = {
48+
name: OBJECT,
49+
label: 'Refusal Cost Doc',
50+
publicSharing: { enabled: true, allowedAudiences: ['public', 'signed_in', 'email'], allowedPermissions: ['view'] },
51+
fields: {
52+
id: { name: 'id', label: 'ID', type: 'text', primaryKey: true },
53+
title: { name: 'title', label: 'Title', type: 'text' },
54+
},
55+
};
56+
57+
let engine: ObjectQL;
58+
let svc: ShareLinkService;
59+
let tokenPw = '';
60+
let hashReads = 0;
61+
let verifyCalls = 0;
62+
63+
/** Run one resolve and return how many hash reads / verifies it performed. */
64+
async function cost(opts: { email?: string; password?: string }): Promise<{
65+
result: unknown;
66+
hashReads: number;
67+
verifyCalls: number;
68+
}> {
69+
hashReads = 0;
70+
verifyCalls = 0;
71+
const result = await svc.resolveToken(tokenPw, {
72+
recipientEmail: opts.email,
73+
providedPassword: opts.password,
74+
});
75+
return { result, hashReads, verifyCalls };
76+
}
77+
78+
beforeAll(async () => {
79+
engine = new ObjectQL();
80+
engine.registerDriver(
81+
new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) as any,
82+
true,
83+
);
84+
await engine.init();
85+
for (const o of [SysShareLink, TARGET]) engine.registry.registerObject(o as any, '@objectstack/plugin-sharing-test');
86+
await engine.syncSchemas();
87+
await engine.insert(OBJECT, { id: RECORD, title: 'Shared doc' }, { context: { isSystem: true } } as any);
88+
89+
// Count the privileged hash read — the `password_hash` dereference the
90+
// internal-column strip forces through the engine's accessor.
91+
const realResolve = (engine as any).resolveInternalField.bind(engine);
92+
(engine as any).resolveInternalField = async (object: string, ids: readonly string[], field: string) => {
93+
if (object === 'sys_share_link' && field === 'password_hash') hashReads++;
94+
return realResolve(object, ids, field);
95+
};
96+
97+
// Count the password verify, wrapping the real verifier so behaviour is intact.
98+
svc = new ShareLinkService({
99+
engine: engine as any,
100+
verifyPassword: (plain, hash) => {
101+
verifyCalls++;
102+
return verifyShareLinkPassword(plain, hash);
103+
},
104+
});
105+
const creator = { isSystem: true, userId: 'usr_creator' } as any;
106+
tokenPw = (
107+
await svc.createLink(
108+
{ object: OBJECT, recordId: RECORD, audience: 'email', password: PASSWORD, emailAllowlist: [ALLOWLISTED] },
109+
creator,
110+
)
111+
).token;
112+
}, 60_000);
113+
114+
afterAll(async () => {
115+
try { await engine?.destroy(); } catch { /* noop */ }
116+
});
117+
118+
describe('[#23474] email-audience refusal cost is input-independent', () => {
119+
it('a wrong password: off-list and allowlisted read the hash and verify equally', async () => {
120+
const offList = await cost({ email: OFFLIST, password: WRONG });
121+
const allowlisted = await cost({ email: ALLOWLISTED, password: WRONG });
122+
// Both refuse (the generic null).
123+
expect(offList.result).toBeNull();
124+
expect(allowlisted.result).toBeNull();
125+
// The membership channel: equal work either way.
126+
expect(allowlisted.hashReads, 'hash reads (wrong password)').toBe(offList.hashReads);
127+
expect(allowlisted.verifyCalls, 'verify calls (wrong password)').toBe(offList.verifyCalls);
128+
// And it is actual work, not equal-at-zero.
129+
expect(offList.hashReads).toBeGreaterThan(0);
130+
expect(offList.verifyCalls).toBeGreaterThan(0);
131+
});
132+
133+
it('no password: off-list and allowlisted read the hash equally (and neither verifies)', async () => {
134+
const offList = await cost({ email: OFFLIST });
135+
const allowlisted = await cost({ email: ALLOWLISTED });
136+
expect(offList.result).toBeNull();
137+
expect(allowlisted.result).toBeNull();
138+
// The hash read is equal too.
139+
expect(allowlisted.hashReads, 'hash reads (no password)').toBe(offList.hashReads);
140+
expect(offList.hashReads).toBeGreaterThan(0);
141+
// No password presented ⇒ no verify on either (a caller learns only about
142+
// its own input, identical for every address).
143+
expect(allowlisted.verifyCalls, 'verify calls (no password)').toBe(offList.verifyCalls);
144+
expect(offList.verifyCalls).toBe(0);
145+
});
146+
147+
it('control: an allowlisted email with the correct password still serves the record', async () => {
148+
const served = await cost({ email: ALLOWLISTED, password: PASSWORD });
149+
expect(served.result).not.toBeNull();
150+
expect((served.result as any)?.link?.record_id).toBe(RECORD);
151+
expect(served.hashReads).toBeGreaterThan(0);
152+
expect(served.verifyCalls).toBeGreaterThan(0);
153+
});
154+
});

‎packages/plugins/plugin-sharing/src/share-link-service.ts‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -796,12 +796,24 @@ export class ShareLinkService implements IShareLinkService {
796796
if (row.expires_at && Date.parse(row.expires_at) <= Date.now()) return null;
797797

798798
// Audience gating.
799+
//
800+
// [#23328] `signed_in`'s early return is kept: it reveals only whether the
801+
// CALLER is signed in, which the caller already knows, so the work it skips
802+
// leaks nothing about a server-side secret.
799803
if (row.audience === 'signed_in' && !probe.signedInUserId) return null;
800-
if (row.audience === 'email') {
801-
const allow = row.email_allowlist ?? [];
802-
const supplied = (probe.recipientEmail ?? '').trim().toLowerCase();
803-
if (!supplied || !allow.includes(supplied)) return null;
804-
}
804+
// [#23474] `email`'s verdict is computed here and combined with the
805+
// password verdict below, so an `email` link's refusal does the same work
806+
// whatever email and password are presented, as its answer bytes are
807+
// already the same (#23372). Scoped to `email`: it is the audience whose
808+
// satisfaction is a server-side list the caller supplies a guess against.
809+
const audiencePassed =
810+
row.audience === 'email'
811+
? (() => {
812+
const allow = row.email_allowlist ?? [];
813+
const supplied = (probe.recipientEmail ?? '').trim().toLowerCase();
814+
return Boolean(supplied) && allow.includes(supplied);
815+
})()
816+
: true;
805817

806818
// [#21197] `password_hash` is `internal`, so the row comes back without
807819
// it, and reading `row.password_hash` would read EVERY link as
@@ -817,14 +829,23 @@ export class ShareLinkService implements IShareLinkService {
817829
// current one — but only once every later gate has passed (below), so a
818830
// switched-off, gone or ineligible link takes no write.
819831
let upgradePasswordFrom: string | undefined;
832+
let passwordPassed = true;
820833
if (passwordHash) {
821-
if (!probe.providedPassword) return null;
822-
const ok = await this.verifyPassword(probe.providedPassword, String(passwordHash));
823-
if (!ok) return null;
824-
if (this.upgradesLegacyPasswordHashes && isLegacyShareLinkPasswordHash(passwordHash)) {
825-
upgradePasswordFrom = probe.providedPassword;
834+
if (!probe.providedPassword) {
835+
passwordPassed = false;
836+
} else {
837+
const ok = await this.verifyPassword(probe.providedPassword, String(passwordHash));
838+
if (!ok) {
839+
passwordPassed = false;
840+
} else if (audiencePassed && this.upgradesLegacyPasswordHashes && isLegacyShareLinkPasswordHash(passwordHash)) {
841+
// The upgrade write stays on the fully-passed path only.
842+
upgradePasswordFrom = probe.providedPassword;
843+
}
826844
}
827845
}
846+
// [#23474] A link serves only when its audience AND its password both
847+
// passed; otherwise the same `null`.
848+
if (!audiencePassed || !passwordPassed) return null;
828849

829850
// [commit fc9ba76a5] The object's policy is read HERE, before the record probe,
830851
// because it decides that probe's projection. `redactFields` is still

0 commit comments

Comments
 (0)