Skip to content

Commit 60dc3ba

Browse files
os-zhuangclaude
andauthored
feat(protocol): enforce the engines.protocol handshake (ADR-0087 P0) (#2650)
* feat(protocol): enforce the engines.protocol handshake (ADR-0087 P0) Turn a protocol/consumer version mismatch from an arbitrary downstream crash into a structured, machine-actionable load-time refusal, and pay down a standing ADR-0078 violation: engines.protocol (ADR-0025 §3.2, protocol-first per §3.10 #3) was declared, documented, and checked by no loader or installer. - spec: export PROTOCOL_VERSION / PROTOCOL_MAJOR from /kernel — the single source of truth the handshake checks against; a drift test keeps it in lockstep with the package major. - metadata-core: checkProtocolCompat() (pure, major-grained range check supporting ^/~/>=/</ranges/wildcards), assertProtocolCompat(), and the structured ProtocolIncompatibleError (OS_PROTOCOL_INCOMPATIBLE, carrying both versions and the 'migrate meta --from N' command). Refuses only on a positive mismatch; absent ranges are grandfathered (warn), unrecognized ranges never cause a false rejection. - metadata-protocol: installPackage runs the handshake before the registry write — incompatible packages are refused with a diagnostic, not a crash. Additive and backward compatible; api-surface snapshot regenerated (2 added, 0 breaking). Part of #2643; resolves #2644. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HjDghc79qFSJkJt2zvBxtU * fix(protocol): make the protocol-range parser ReDoS-safe (CodeQL 837/838) The comparator match (`^(<=|>=|<|>)\s*(.+)$`) and the hyphen-range match (`^(.+?)\s+-\s+(.+)$`) had whitespace/any-char overlap — polynomial-ReDoS shapes on the externally-authored engines string. - comparator: peel the operator by fixed prefix + trim, no backtracking match - hyphen range: split on the whitespace-delimited hyphen (fixed anchor), not a lazy (.+?)…(.+) match - bound the range string to 128 chars up front (a real SemVer range is short; overlong input is unrecognized = admit-with-warning, never a slow scan) Behavior unchanged (100 tests green); adds a ReDoS-regression test asserting pathological inputs resolve to null in well under 50ms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HjDghc79qFSJkJt2zvBxtU --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1dd5dfd commit 60dc3ba

10 files changed

Lines changed: 643 additions & 1 deletion

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/metadata-core": minor
4+
"@objectstack/metadata-protocol": minor
5+
---
6+
7+
ADR-0087 P0 — enforce the protocol version handshake (make `engines.protocol` real).
8+
9+
`PluginEnginesSchema.protocol` (ADR-0025 §3.2, protocol-first per §3.10 #3) was declared, documented, and checked by no loader or installer — an ADR-0078 "declarable-but-inert" violation. A package built against an incompatible protocol major failed deep in a schema `.parse()` or a renderer contract instead of at the boundary.
10+
11+
- **`@objectstack/spec`**: exports `PROTOCOL_VERSION` / `PROTOCOL_MAJOR` (`kernel`) — the single source of truth the handshake checks against. A drift test keeps it in lockstep with the package major.
12+
- **`@objectstack/metadata-core`**: adds `checkProtocolCompat()` (pure, major-grained range check), `assertProtocolCompat()`, and the structured `ProtocolIncompatibleError` (`OS_PROTOCOL_INCOMPATIBLE`, carrying both versions and the `objectstack migrate meta --from N` command). It refuses only on a *positive* mismatch determination; absent ranges are grandfathered (warn) and unrecognized ranges never cause a false rejection.
13+
- **`@objectstack/metadata-protocol`**: `installPackage` runs the handshake before writing to the registry — an incompatible package is refused with a machine-actionable diagnostic instead of crashing later.
14+
15+
Additive and backward compatible: packages that declare no `engines.protocol` range keep loading (with a warning). Part of the ADR-0087 epic (#2643); resolves #2644.

packages/metadata-core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,5 @@ export * from './repository.js';
1313
export * from './in-memory-repository.js';
1414
export * from './cache.js';
1515
export * from './layered-repository.js';
16+
export * from './protocol-handshake.js';
1617
export * from './objects/index.js';
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, expect, it, vi } from 'vitest';
4+
import {
5+
assertProtocolCompat,
6+
checkProtocolCompat,
7+
ProtocolIncompatibleError,
8+
rangeAdmitsMajor,
9+
} from './protocol-handshake.js';
10+
11+
const RT = '11.0.0'; // runtime protocol version used across these tests
12+
13+
describe('rangeAdmitsMajor', () => {
14+
it('caret pins the major', () => {
15+
expect(rangeAdmitsMajor('^11', 11)).toBe(true);
16+
expect(rangeAdmitsMajor('^11.2.0', 11)).toBe(true);
17+
expect(rangeAdmitsMajor('^10', 11)).toBe(false);
18+
expect(rangeAdmitsMajor('^12', 11)).toBe(false);
19+
});
20+
21+
it('tilde pins the major', () => {
22+
expect(rangeAdmitsMajor('~11.4.0', 11)).toBe(true);
23+
expect(rangeAdmitsMajor('~10.9.9', 11)).toBe(false);
24+
});
25+
26+
it('bare exact / bare major', () => {
27+
expect(rangeAdmitsMajor('11', 11)).toBe(true);
28+
expect(rangeAdmitsMajor('11.0.0', 11)).toBe(true);
29+
expect(rangeAdmitsMajor('10', 11)).toBe(false);
30+
});
31+
32+
it('wildcard forms', () => {
33+
expect(rangeAdmitsMajor('*', 11)).toBe(true);
34+
expect(rangeAdmitsMajor('latest', 11)).toBe(true);
35+
expect(rangeAdmitsMajor('11.x', 11)).toBe(true);
36+
expect(rangeAdmitsMajor('10.x', 11)).toBe(false);
37+
});
38+
39+
it('single comparators', () => {
40+
expect(rangeAdmitsMajor('>=11', 11)).toBe(true);
41+
expect(rangeAdmitsMajor('>=12', 11)).toBe(false);
42+
expect(rangeAdmitsMajor('>=10.0.0', 11)).toBe(true);
43+
expect(rangeAdmitsMajor('<13', 11)).toBe(true);
44+
expect(rangeAdmitsMajor('<11', 11)).toBe(false);
45+
expect(rangeAdmitsMajor('>11', 11)).toBe(false); // bare major excludes 11
46+
expect(rangeAdmitsMajor('>11', 12)).toBe(true);
47+
});
48+
49+
it('compound comparator ranges', () => {
50+
expect(rangeAdmitsMajor('>=11.0.0 <13.0.0', 11)).toBe(true);
51+
expect(rangeAdmitsMajor('>=11.0.0 <13.0.0', 12)).toBe(true);
52+
expect(rangeAdmitsMajor('>=11.0.0 <13.0.0', 13)).toBe(false);
53+
expect(rangeAdmitsMajor('>=12 <14', 11)).toBe(false);
54+
});
55+
56+
it('hyphen ranges', () => {
57+
expect(rangeAdmitsMajor('10.0.0 - 12.0.0', 11)).toBe(true);
58+
expect(rangeAdmitsMajor('10.0.0 - 12.0.0', 13)).toBe(false);
59+
});
60+
61+
it('returns null for unrecognized shapes (never a false rejection)', () => {
62+
expect(rangeAdmitsMajor('', 11)).toBeNull();
63+
expect(rangeAdmitsMajor('garbage', 11)).toBeNull();
64+
expect(rangeAdmitsMajor('workspace:*', 11)).toBeNull();
65+
});
66+
67+
it('bounds pathological input (ReDoS-safe) without a slow scan', () => {
68+
// The engines string is externally authored; the comparator/hyphen parsing
69+
// must not degrade on adversarial input (CodeQL alerts 837/838).
70+
const overlong = '<' + '\t'.repeat(100_000);
71+
const hyphenBomb = 'a\t-\t' + '\t'.repeat(100_000);
72+
const start = performance.now();
73+
expect(rangeAdmitsMajor(overlong, 11)).toBeNull();
74+
expect(rangeAdmitsMajor(hyphenBomb, 11)).toBeNull();
75+
expect(rangeAdmitsMajor('>=11.0.0 ' + ' '.repeat(100_000) + '<13.0.0', 11)).toBeNull();
76+
expect(performance.now() - start).toBeLessThan(50);
77+
});
78+
});
79+
80+
describe('checkProtocolCompat', () => {
81+
it('ok when the declared protocol range admits the runtime major', () => {
82+
const r = checkProtocolCompat({ id: 'a', engines: { protocol: '^11' } }, RT);
83+
expect(r.status).toBe('ok');
84+
});
85+
86+
it('incompatible with a structured diagnostic when it does not', () => {
87+
const r = checkProtocolCompat({ id: 'com.acme.crm', engines: { protocol: '^10' } }, RT);
88+
expect(r.status).toBe('incompatible');
89+
if (r.status !== 'incompatible') return;
90+
expect(r.diagnostic.code).toBe('OS_PROTOCOL_INCOMPATIBLE');
91+
expect(r.diagnostic.packageId).toBe('com.acme.crm');
92+
expect(r.diagnostic.requiredRange).toBe('^10');
93+
expect(r.diagnostic.rangeSource).toBe('engines.protocol');
94+
expect(r.diagnostic.runtimeVersion).toBe(RT);
95+
expect(r.diagnostic.targetMajor).toBe(10);
96+
expect(r.diagnostic.migrateCommand).toBe('objectstack migrate meta --from 10');
97+
// The message names both versions and the command — the whole point of D1.
98+
expect(r.diagnostic.message).toContain('^10');
99+
expect(r.diagnostic.message).toContain('11.0.0');
100+
expect(r.diagnostic.message).toContain('migrate meta --from 10');
101+
});
102+
103+
it('the diagnostic is identical in shape one major behind or five', () => {
104+
const near = checkProtocolCompat({ id: 'p', engines: { protocol: '^10' } }, RT);
105+
const far = checkProtocolCompat({ id: 'p', engines: { protocol: '^6' } }, RT);
106+
expect(near.status).toBe('incompatible');
107+
expect(far.status).toBe('incompatible');
108+
if (near.status !== 'incompatible' || far.status !== 'incompatible') return;
109+
expect(Object.keys(near.diagnostic).sort()).toEqual(Object.keys(far.diagnostic).sort());
110+
expect(far.diagnostic.migrateCommand).toBe('objectstack migrate meta --from 6');
111+
});
112+
113+
it('protocol-first precedence over platform and legacy engine', () => {
114+
// protocol wins even when platform/legacy would say otherwise
115+
const r = checkProtocolCompat(
116+
{ id: 'p', engines: { protocol: '^11', platform: '^10' }, engine: { objectstack: '^9' } },
117+
RT,
118+
);
119+
expect(r.status).toBe('ok');
120+
if (r.status !== 'ok') return;
121+
expect(r.source).toBe('engines.protocol');
122+
});
123+
124+
it('falls back to platform, then legacy engine.objectstack', () => {
125+
const viaPlatform = checkProtocolCompat({ id: 'p', engines: { platform: '^11' } }, RT);
126+
expect(viaPlatform.status).toBe('ok');
127+
if (viaPlatform.status === 'ok') expect(viaPlatform.source).toBe('engines.platform');
128+
129+
const viaLegacy = checkProtocolCompat({ id: 'p', engine: { objectstack: '>=10' } }, RT);
130+
expect(viaLegacy.status).toBe('ok');
131+
if (viaLegacy.status === 'ok') expect(viaLegacy.source).toBe('engine.objectstack');
132+
});
133+
134+
it('no-range when nothing is declared (grandfathering)', () => {
135+
expect(checkProtocolCompat({ id: 'p' }, RT).status).toBe('no-range');
136+
expect(checkProtocolCompat({ id: 'p', engines: {} }, RT).status).toBe('no-range');
137+
});
138+
139+
it('unparsed-range for a present but unrecognized range (no false rejection)', () => {
140+
const r = checkProtocolCompat({ id: 'p', engines: { protocol: 'workspace:*' } }, RT);
141+
expect(r.status).toBe('unparsed-range');
142+
});
143+
});
144+
145+
describe('assertProtocolCompat', () => {
146+
it('throws ProtocolIncompatibleError on a positive mismatch', () => {
147+
expect(() =>
148+
assertProtocolCompat({ id: 'p', engines: { protocol: '^10' } }, RT, () => {}),
149+
).toThrow(ProtocolIncompatibleError);
150+
try {
151+
assertProtocolCompat({ id: 'p', engines: { protocol: '^10' } }, RT, () => {});
152+
} catch (e) {
153+
expect(e).toBeInstanceOf(ProtocolIncompatibleError);
154+
expect((e as ProtocolIncompatibleError).code).toBe('OS_PROTOCOL_INCOMPATIBLE');
155+
expect((e as ProtocolIncompatibleError).diagnostic.migrateCommand).toContain('--from 10');
156+
}
157+
});
158+
159+
it('returns silently on ok', () => {
160+
const warn = vi.fn();
161+
expect(() => assertProtocolCompat({ id: 'p', engines: { protocol: '^11' } }, RT, warn)).not.toThrow();
162+
expect(warn).not.toHaveBeenCalled();
163+
});
164+
165+
it('warns (does not throw) on no-range', () => {
166+
const warn = vi.fn();
167+
assertProtocolCompat({ id: 'p' }, RT, warn);
168+
expect(warn).toHaveBeenCalledOnce();
169+
expect(warn.mock.calls[0]![0]).toContain('no engines.protocol range');
170+
});
171+
172+
it('warns (does not throw) on an unparsed range', () => {
173+
const warn = vi.fn();
174+
assertProtocolCompat({ id: 'p', engines: { protocol: '???' } }, RT, warn);
175+
expect(warn).toHaveBeenCalledOnce();
176+
expect(warn.mock.calls[0]![0]).toContain('unrecognized');
177+
});
178+
});

0 commit comments

Comments
 (0)