Skip to content

Commit f97166f

Browse files
sorccuclaude
sorccu
and
claude
committed
refactor(cli): one operator class per SSL assertion property
certificate(p)/connection(p) returned a GeneralAssertionBuilder exposing every operator with string|number|boolean targets, so certificate('daysUntilExpiry').greaterThan('30 days') compiled, passed deploy validation, and then silently never fired. Give each property its own operator class exposing exactly the comparisons the backend accepts for it, with its target typed. The operator maps are the declaration point: the property unions derive from their keys, and the value-type and comparison tables are keyed by those unions, so a property cannot be added to one and forgotten in another. An unknown property still resolves, and is reported by validation rather than throwing. Validation now reports a non-numeric target for numeric properties and for RESPONSE_TIME, closing the same silent-never-fires hole on the object-literal path that bypasses the builder. Targets are checked only where the accepted set is universal; the enumerated sets stay with the builder's types to avoid restating the value unions. Codegen emits numeric and boolean targets as bare literals so the generated call matches the operators, sharing its numeric predicate with validation so nothing the CLI calls valid is emitted as code that does not compile. Targets outside a property's contract keep their string form rather than being coerced, which would rewrite the assertion on the next deploy. valueForNumericAssertion keeps parseInt for its other callers, whose targets may carry units; SSL opts into Number via the new parse option. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7a7d6ce commit f97166f

9 files changed

Lines changed: 824 additions & 112 deletions

‎packages/cli/src/constructs/__tests__/ssl-assertion-codegen.spec.ts‎

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,23 @@ describe('SSL Assertion Codegen', () => {
1616
it('generates the property-scoped SSL assertion sources', () => {
1717
const cases: { input: SslAssertion, expected: string }[] = [
1818
// CERTIFICATE / CONNECTION emit the property name as the first call argument.
19+
// Numeric properties emit an unquoted number, matching the builder's typed target.
1920
{
2021
input: { source: 'CERTIFICATE', property: 'daysUntilExpiry', comparison: 'GREATER_THAN', target: '30', regex: null },
21-
expected: 'SslAssertionBuilder.certificate(\'daysUntilExpiry\').greaterThan(\'30\')\n',
22+
expected: 'SslAssertionBuilder.certificate(\'daysUntilExpiry\').greaterThan(30)\n',
2223
},
2324
{
2425
input: { source: 'CERTIFICATE', property: 'signatureAlgorithm', comparison: 'EQUALS', target: 'SHA256-RSA', regex: null },
2526
expected: 'SslAssertionBuilder.certificate(\'signatureAlgorithm\').equals(\'SHA256-RSA\')\n',
2627
},
28+
// Boolean properties emit a boolean literal, not a quoted string.
2729
{
2830
input: { source: 'CERTIFICATE', property: 'selfSigned', comparison: 'EQUALS', target: 'false', regex: null },
29-
expected: 'SslAssertionBuilder.certificate(\'selfSigned\').equals(\'false\')\n',
31+
expected: 'SslAssertionBuilder.certificate(\'selfSigned\').equals(false)\n',
32+
},
33+
{
34+
input: { source: 'CONNECTION', property: 'chainTrusted', comparison: 'EQUALS', target: 'true', regex: null },
35+
expected: 'SslAssertionBuilder.connection(\'chainTrusted\').equals(true)\n',
3036
},
3137
{
3238
input: { source: 'CONNECTION', property: 'tlsVersion', comparison: 'EQUALS', target: 'TLS1.3', regex: null },
@@ -56,4 +62,70 @@ describe('SSL Assertion Codegen', () => {
5662
expect(render(test.input)).toEqual(test.expected)
5763
}
5864
})
65+
66+
// The backend accepts any target string, so a monitor created via the UI or API can
67+
// carry a target the builder's typed operators cannot express. Rendering it as a typed
68+
// literal anyway would rewrite the assertion on the next deploy ('yes' becoming false,
69+
// '30.5' becoming 30), so these fall back to the string form, which preserves the value.
70+
it('does not coerce a target its typed operators cannot express', () => {
71+
const cases: { input: SslAssertion, expected: string }[] = [
72+
{
73+
input: { source: 'CERTIFICATE', property: 'selfSigned', comparison: 'EQUALS', target: 'yes', regex: null },
74+
expected: 'SslAssertionBuilder.certificate(\'selfSigned\').equals(\'yes\')\n',
75+
},
76+
{
77+
input: { source: 'CERTIFICATE', property: 'keySizeBits', comparison: 'EQUALS', target: '', regex: null },
78+
expected: 'SslAssertionBuilder.certificate(\'keySizeBits\').equals(\'\')\n',
79+
},
80+
{
81+
input: { source: 'CERTIFICATE', property: 'daysUntilExpiry', comparison: 'GREATER_THAN', target: 'abc', regex: null },
82+
expected: 'SslAssertionBuilder.certificate(\'daysUntilExpiry\').greaterThan(\'abc\')\n',
83+
},
84+
{
85+
// parseInt would take the leading prefix and silently assert against 5.
86+
input: { source: 'CERTIFICATE', property: 'keySizeBits', comparison: 'EQUALS', target: '5%', regex: null },
87+
expected: 'SslAssertionBuilder.certificate(\'keySizeBits\').equals(\'5%\')\n',
88+
},
89+
]
90+
for (const test of cases) {
91+
expect(render(test.input)).toEqual(test.expected)
92+
}
93+
})
94+
95+
// Codegen must render every target validateSslAssertion accepts as a numeric literal:
96+
// the property's operators take a number, so a quoted target would not compile. parseInt
97+
// would truncate the fractional and exponent forms.
98+
it('renders every numeric target validation accepts as a number literal', () => {
99+
const cases: { target: string, expected: string }[] = [
100+
{ target: '30', expected: '30' },
101+
{ target: '30.5', expected: '30.5' },
102+
{ target: '0.5', expected: '0.5' },
103+
{ target: '1e3', expected: '1000' },
104+
{ target: '-5', expected: '-5' },
105+
{ target: ' 30 ', expected: '30' },
106+
]
107+
for (const { target, expected } of cases) {
108+
expect(render(
109+
{ source: 'CERTIFICATE', property: 'daysUntilExpiry', comparison: 'GREATER_THAN', target, regex: null },
110+
)).toEqual(`SslAssertionBuilder.certificate('daysUntilExpiry').greaterThan(${expected})\n`)
111+
}
112+
})
113+
114+
// A comparison outside a typed helper's set makes the helper throw, which would abort
115+
// the whole import over one assertion.
116+
it('renders rather than throws on a comparison the property does not support', () => {
117+
expect(render(
118+
{ source: 'CERTIFICATE', property: 'daysUntilExpiry', comparison: 'CONTAINS', target: '30', regex: null },
119+
)).toEqual('SslAssertionBuilder.certificate(\'daysUntilExpiry\').contains(\'30\')\n')
120+
expect(render(
121+
{ source: 'CERTIFICATE', property: 'selfSigned', comparison: 'NOT_EQUALS', target: 'true', regex: null },
122+
)).toEqual('SslAssertionBuilder.certificate(\'selfSigned\').notEquals(\'true\')\n')
123+
})
124+
125+
// An unknown property reaches codegen from remote data; validation reports it.
126+
it('renders an unknown property rather than throwing', () => {
127+
expect(render(
128+
{ source: 'CERTIFICATE', property: 'bogusProperty', comparison: 'EQUALS', target: 'x', regex: null },
129+
)).toEqual('SslAssertionBuilder.certificate(\'bogusProperty\').equals(\'x\')\n')
130+
})
59131
})

‎packages/cli/src/constructs/__tests__/ssl-assertion.spec.ts‎

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,16 @@ describe('SslAssertionBuilder', () => {
4949
source: 'CERTIFICATE', property: 'isCA', comparison: 'EQUALS', target: 'true',
5050
})
5151
})
52+
53+
// Numeric and boolean targets are narrowed at compile time, so the wire payload is
54+
// the only place a wrong-typed target could still surface. These run under vitest
55+
// and hold regardless of whether the type-level cases above are ever checked.
56+
it('stringifies typed targets onto the wire', () => {
57+
expect(SslAssertionBuilder.certificate('daysUntilExpiry').greaterThan(30).target).toEqual('30')
58+
expect(SslAssertionBuilder.certificate('keySizeBits').equals(2048).target).toEqual('2048')
59+
expect(SslAssertionBuilder.certificate('selfSigned').equals(false).target).toEqual('false')
60+
expect(SslAssertionBuilder.connection('chainTrusted').equals(true).target).toEqual('true')
61+
})
5262
})
5363

5464
describe('connection(property)', () => {
@@ -113,9 +123,10 @@ describe('SslAssertionBuilder', () => {
113123
})
114124
})
115125

116-
// These cases assert compile-time narrowing. The `@ts-expect-error` lines are the
117-
// real assertions: `tsc --build` fails if any becomes a false positive (an unused
118-
// expect-error). The runtime `expect` keeps the block a valid test.
126+
// The `@ts-expect-error` lines below document the intended narrowing but currently
127+
// assert nothing: tsconfig.json excludes `src/**/__tests__` and vitest does not
128+
// type-check, so no build step ever evaluates them. RED-739 tracks wiring that up.
129+
// Until it lands, only the runtime `expect` calls here are enforced.
119130
describe('type narrowing (compile-time)', () => {
120131
it('constrains connection(tlsVersion) targets to TlsVersionValue', () => {
121132
expect(SslAssertionBuilder.connection('tlsVersion').equals(TlsVersion.TLS1_3)).toMatchObject({
@@ -140,6 +151,54 @@ describe('SslAssertionBuilder', () => {
140151
SslAssertionBuilder.connection('bogusProperty')
141152
})
142153

154+
it('constrains numeric certificate properties to number targets', () => {
155+
// @ts-expect-error a numeric property does not take a string target
156+
SslAssertionBuilder.certificate('daysUntilExpiry').greaterThan('30 days')
157+
// @ts-expect-error a numeric property does not take a boolean target
158+
SslAssertionBuilder.certificate('keySizeBits').equals(true)
159+
})
160+
161+
it('constrains boolean properties to boolean targets', () => {
162+
// @ts-expect-error a boolean property does not take a string target
163+
SslAssertionBuilder.certificate('selfSigned').equals('yes')
164+
// @ts-expect-error a boolean property does not take a string target
165+
SslAssertionBuilder.connection('chainTrusted').equals('yes')
166+
})
167+
168+
// Each property exposes only the operators the backend accepts for its value type,
169+
// so an unsupported comparison is a compile error rather than a deploy diagnostic.
170+
it('offers only the operators a property supports', () => {
171+
// These operators do not exist at runtime, so the body is type-checked but never
172+
// executed (an uncalled function) — calling it would throw a TypeError.
173+
const _typeChecks = () => {
174+
// @ts-expect-error a boolean property supports EQUALS only
175+
SslAssertionBuilder.certificate('selfSigned').notEquals(true)
176+
// @ts-expect-error a numeric property is not substring-matched
177+
SslAssertionBuilder.certificate('keySizeBits').contains('20')
178+
// @ts-expect-error an opaque identifier is not substring-matched
179+
SslAssertionBuilder.certificate('fingerprintSha256').contains('ab')
180+
// @ts-expect-error a string list has no whole-value to compare against
181+
SslAssertionBuilder.certificate('sans').equals('example.com')
182+
// @ts-expect-error an enum is not ordered
183+
SslAssertionBuilder.certificate('signatureAlgorithm').greaterThan('SHA256-RSA')
184+
// @ts-expect-error a free-form string is not ordered
185+
SslAssertionBuilder.connection('cipherSuite').greaterThan('TLS_AES_256_GCM_SHA384')
186+
}
187+
expect(_typeChecks).toBeDefined()
188+
})
189+
190+
it('allows the operators a property does support', () => {
191+
expect(SslAssertionBuilder.connection('tlsVersion').greaterThan(TlsVersion.TLS1_2)).toMatchObject({
192+
source: 'CONNECTION', property: 'tlsVersion', comparison: 'GREATER_THAN', target: 'TLS1.2',
193+
})
194+
expect(SslAssertionBuilder.certificate('sans').notContains('evil.example.com')).toMatchObject({
195+
source: 'CERTIFICATE', property: 'sans', comparison: 'NOT_CONTAINS', target: 'evil.example.com',
196+
})
197+
expect(SslAssertionBuilder.certificate('serialNumber').notEquals('ab12')).toMatchObject({
198+
source: 'CERTIFICATE', property: 'serialNumber', comparison: 'NOT_EQUALS', target: 'ab12',
199+
})
200+
})
201+
143202
it('leaves cipherSuite targets unconstrained (arbitrary string)', () => {
144203
expect(SslAssertionBuilder.connection('cipherSuite').equals('SOME_FUTURE_SUITE')).toMatchObject({
145204
source: 'CONNECTION', property: 'cipherSuite', comparison: 'EQUALS', target: 'SOME_FUTURE_SUITE',

‎packages/cli/src/constructs/__tests__/ssl-monitor.spec.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,81 @@ describe('SslMonitor', () => {
338338
]))
339339
})
340340

341+
it('should error on a numeric property with a non-numeric target', async () => {
342+
// The builder types these targets as numbers, but an object literal bypasses it and
343+
// the backend accepts the string — it just never matches, so the assertion would
344+
// silently never fire.
345+
const cases: Array<{ target: string, expected: string }> = [
346+
{ target: '30 days', expected: 'must compare against a number, but got "30 days".' },
347+
{ target: '', expected: 'must compare against a number, but got (none).' },
348+
{ target: 'abc', expected: 'must compare against a number, but got "abc".' },
349+
]
350+
for (const { target, expected } of cases) {
351+
const diags = await validateWith([
352+
{ source: 'CERTIFICATE', property: 'daysUntilExpiry', comparison: 'GREATER_THAN', target, regex: null },
353+
])
354+
expect(diags.isFatal()).toEqual(true)
355+
expect(diags.observations).toEqual(expect.arrayContaining([
356+
expect.objectContaining({
357+
message: expect.stringContaining(
358+
`The CERTIFICATE "daysUntilExpiry" assertion at "request.assertions[0]" ${expected}`,
359+
),
360+
}),
361+
]))
362+
}
363+
})
364+
365+
it('should error on a non-numeric RESPONSE_TIME target', async () => {
366+
const diags = await validateWith([
367+
{ source: 'RESPONSE_TIME', property: '', comparison: 'LESS_THAN', target: '1s', regex: null },
368+
])
369+
expect(diags.isFatal()).toEqual(true)
370+
expect(diags.observations).toEqual(expect.arrayContaining([
371+
expect.objectContaining({
372+
message: expect.stringContaining(
373+
'The RESPONSE_TIME assertion at "request.assertions[0]" must compare against a number, but got "1s".',
374+
),
375+
}),
376+
]))
377+
})
378+
379+
// Check files are loaded without typechecking, so an object-literal assertion can
380+
// carry a non-string target. It must be reported, not thrown on.
381+
it('should report rather than crash on a non-string target', async () => {
382+
const cases: unknown[] = [
383+
{ source: 'CERTIFICATE', property: 'daysUntilExpiry', comparison: 'EQUALS', target: 30, regex: null },
384+
{ source: 'CONNECTION', property: 'chainTrusted', comparison: 'EQUALS', target: true, regex: null },
385+
{ source: 'CERTIFICATE', property: 'daysUntilExpiry', comparison: 'EQUALS', regex: null },
386+
]
387+
for (const assertion of cases) {
388+
const diags = await validateWith([assertion] as SslRequest['assertions'])
389+
expect(diags.isFatal()).toEqual(true)
390+
expect(diags.observations).toEqual(expect.arrayContaining([
391+
expect.objectContaining({ message: expect.stringContaining('written as a string') }),
392+
]))
393+
}
394+
})
395+
396+
// A target that is never inspected must not crash either, whatever its type.
397+
it('should not crash on assertions whose target it does not inspect', async () => {
398+
const diags = await validateWith([
399+
{ source: 'TEXT_RESPONSE', property: '', comparison: 'NOT_EMPTY', regex: null },
400+
{ source: 'JSON_RESPONSE', property: '$.x', comparison: 'IS_NULL', target: null, regex: null },
401+
{ source: 'CERTIFICATE', property: 'issuerCN', comparison: 'CONTAINS', target: 'CA', regex: null },
402+
] as unknown as SslRequest['assertions'])
403+
expect(diags.isFatal()).toEqual(false)
404+
})
405+
406+
it('should accept the numeric targets a numeric property does allow', async () => {
407+
const diags = await validateWith([
408+
{ source: 'CERTIFICATE', property: 'daysUntilExpiry', comparison: 'GREATER_THAN', target: '30', regex: null },
409+
{ source: 'CERTIFICATE', property: 'daysUntilExpiry', comparison: 'LESS_THAN', target: '30.5', regex: null },
410+
{ source: 'CERTIFICATE', property: 'keySizeBits', comparison: 'EQUALS', target: '2048', regex: null },
411+
{ source: 'RESPONSE_TIME', property: '', comparison: 'LESS_THAN', target: '1000', regex: null },
412+
])
413+
expect(diags.isFatal()).toEqual(false)
414+
})
415+
341416
it('should error on a boolean property with a non-boolean target', async () => {
342417
const diags = await validateWith([
343418
{ source: 'CONNECTION', property: 'chainTrusted', comparison: 'EQUALS', target: 'yes', regex: null },

‎packages/cli/src/constructs/__tests__/uptime-monitor-codegen.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,14 +207,14 @@ describe('SslMonitorCodegen', () => {
207207
],
208208
},
209209
}))
210-
expect(source).toContain('certificate(\'daysUntilExpiry\').greaterThan(\'20\')')
211-
expect(source).toContain('certificate(\'keySizeBits\').equals(\'2048\')')
210+
expect(source).toContain('certificate(\'daysUntilExpiry\').greaterThan(20)')
211+
expect(source).toContain('certificate(\'keySizeBits\').equals(2048)')
212212
expect(source).toContain('certificate(\'signatureAlgorithm\').equals(\'SHA256-RSA\')')
213213
expect(source).toContain('certificate(\'issuerCN\').contains(\'Example CA\')')
214-
expect(source).toContain('certificate(\'selfSigned\').equals(\'false\')')
214+
expect(source).toContain('certificate(\'selfSigned\').equals(false)')
215215
expect(source).toContain('connection(\'tlsVersion\').equals(\'TLS1.3\')')
216216
expect(source).toContain('connection(\'cipherSuite\').equals(\'TLS_AES_256_GCM_SHA384\')')
217-
expect(source).toContain('connection(\'chainTrusted\').equals(\'true\')')
217+
expect(source).toContain('connection(\'chainTrusted\').equals(true)')
218218
expect(source).toContain('responseTime().lessThan(1000)')
219219
expect(source).toContain('jsonResponse(\'$.status\').equals(\'ok\')')
220220
expect(source).toContain('textResponse().contains(\'healthy\')')

‎packages/cli/src/constructs/internal/assertion-codegen.ts‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ export function unsupportedAssertionSource (source: never, kind?: string): never
1616

1717
export interface ValueForNumericAssertionOptions {
1818
hasProperty?: boolean
19+
/**
20+
* How to read the target as a number. Defaults to `parseInt`, which takes the leading
21+
* numeric prefix — lenient by design, since wire targets may carry units ('5%').
22+
* Callers that have already established the target is a full number can pass `Number`
23+
* to keep fractions and avoid truncating '30.5' to 30.
24+
*/
25+
parse?: (target: string) => number
1926
}
2027

2128
export function valueForNumericAssertion<Source extends string> (
@@ -24,6 +31,7 @@ export function valueForNumericAssertion<Source extends string> (
2431
assertion: Assertion<Source>,
2532
options?: ValueForNumericAssertionOptions,
2633
): Value {
34+
const parse = options?.parse ?? ((target: string) => parseInt(target, 10))
2735
return expr(ident(klass), builder => {
2836
builder.member(ident(method))
2937
builder.call(builder => {
@@ -36,25 +44,58 @@ export function valueForNumericAssertion<Source extends string> (
3644
case 'EQUALS':
3745
builder.member(ident('equals'))
3846
builder.call(builder => {
39-
builder.number(parseInt(assertion.target, 10))
47+
builder.number(parse(assertion.target))
4048
})
4149
break
4250
case 'NOT_EQUALS':
4351
builder.member(ident('notEquals'))
4452
builder.call(builder => {
45-
builder.number(parseInt(assertion.target, 10))
53+
builder.number(parse(assertion.target))
4654
})
4755
break
4856
case 'LESS_THAN':
4957
builder.member(ident('lessThan'))
5058
builder.call(builder => {
51-
builder.number(parseInt(assertion.target, 10))
59+
builder.number(parse(assertion.target))
5260
})
5361
break
5462
case 'GREATER_THAN':
5563
builder.member(ident('greaterThan'))
5664
builder.call(builder => {
57-
builder.number(parseInt(assertion.target, 10))
65+
builder.number(parse(assertion.target))
66+
})
67+
break
68+
default:
69+
throw new Error(`Unsupported comparison ${assertion.comparison} for assertion source ${assertion.source}`)
70+
}
71+
})
72+
}
73+
74+
export interface ValueForBooleanAssertionOptions {
75+
hasProperty?: boolean
76+
}
77+
78+
// Emits a boolean-target assertion: `Builder.method([property]).equals(true|false)`.
79+
// Boolean targets only support EQUALS and carry no regex.
80+
export function valueForBooleanAssertion<Source extends string> (
81+
klass: string,
82+
method: string,
83+
assertion: Assertion<Source>,
84+
options?: ValueForBooleanAssertionOptions,
85+
): Value {
86+
return expr(ident(klass), builder => {
87+
builder.member(ident(method))
88+
builder.call(builder => {
89+
const hasProperty = options?.hasProperty ?? false
90+
if (hasProperty && assertion.property !== '') {
91+
builder.string(assertion.property)
92+
}
93+
})
94+
switch (assertion.comparison) {
95+
case 'EQUALS':
96+
builder.member(ident('equals'))
97+
builder.call(builder => {
98+
builder.boolean(assertion.target === 'true')
5899
})
59100
break
60101
default:

0 commit comments

Comments
 (0)