From 5580bf02916306ef1138dd80f6681d2c3c1d5922 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Mon, 5 Oct 2026 07:40:53 -0300 Subject: [PATCH 1/2] feat(agent-config): overlay validation, config diff and display helpers Layer 5 of 21 in the stacked split of compliance-framework/ui#318. Co-Authored-By: Claude Opus 5.5 --- .../__tests__/config-diff.spec.ts | 119 +++++++++ .../agent-config/__tests__/display.spec.ts | 17 ++ .../agent-config/__tests__/validation.spec.ts | 122 +++++++++ src/utils/agent-config/config-diff.ts | 237 ++++++++++++++++++ src/utils/agent-config/display.ts | 60 +++++ src/utils/agent-config/validation.ts | 152 +++++++++++ 6 files changed, 707 insertions(+) create mode 100644 src/utils/agent-config/__tests__/config-diff.spec.ts create mode 100644 src/utils/agent-config/__tests__/display.spec.ts create mode 100644 src/utils/agent-config/__tests__/validation.spec.ts create mode 100644 src/utils/agent-config/config-diff.ts create mode 100644 src/utils/agent-config/display.ts create mode 100644 src/utils/agent-config/validation.ts diff --git a/src/utils/agent-config/__tests__/config-diff.spec.ts b/src/utils/agent-config/__tests__/config-diff.spec.ts new file mode 100644 index 00000000..bf1660bb --- /dev/null +++ b/src/utils/agent-config/__tests__/config-diff.spec.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from 'vitest'; +import { changedLeafPaths, diffConfigs } from '../config-diff'; + +describe('diffConfigs', () => { + it('recurses into objects and compares arrays and strings whole', () => { + const before = { + verbosity: 0, + plugins: { a: { policies: ['x'], schedule: 's', gone: 1 } }, + }; + const after = { + verbosity: 1, + plugins: { a: { policies: ['x', 'y'], schedule: 's' }, b: {} }, + }; + expect(diffConfigs(before, after)).toEqual([ + { path: '/plugins/a/gone', kind: 'removed', before: 1 }, + { + path: '/plugins/a/policies', + kind: 'changed', + before: ['x'], + after: ['x', 'y'], + }, + { path: '/plugins/b', kind: 'added', after: {} }, + { path: '/verbosity', kind: 'changed', before: 0, after: 1 }, + ]); + }); + + it('flags multi-line strings and escapes pointer tokens', () => { + const d = diffConfigs( + { plugins: { p: { policy_data: { 'd/x': 'line a\n' } } } }, + { plugins: { p: { policy_data: { 'd/x': 'line b\n' } } } }, + ); + expect(d).toEqual([ + { + path: '/plugins/p/policy_data/d~1x', + kind: 'changed', + before: 'line a\n', + after: 'line b\n', + multiline: true, + }, + ]); + }); + + it('returns nothing for equal documents', () => { + expect(diffConfigs({ a: { b: [1] } }, { a: { b: [1] } })).toEqual([]); + expect(diffConfigs(null, {})).toEqual([]); + }); +}); + +describe('changedLeafPaths', () => { + it('lists differing leaves (arrays whole)', () => { + expect( + changedLeafPaths( + { verbosity: 1, plugins: { a: { policies: ['x'] } } }, + { plugins: { a: { policies: ['x', 'y'] }, b: { source: 's' } } }, + ), + ).toEqual(['/plugins/a/policies', '/plugins/b/source', '/verbosity']); + }); + + it('lists an empty object that was added or removed', () => { + const o = { plugins: { p: { policy_data: { n: {} } } } }; + expect(changedLeafPaths({}, o)).toEqual(['/plugins/p/policy_data/n']); + expect(changedLeafPaths(o, {})).toEqual(['/plugins/p/policy_data/n']); + expect(changedLeafPaths(o, o)).toEqual([]); + }); +}); + +describe('element-level array changes', () => { + it('pairs a replaced item, and keeps insertions / removals from shifting the rest', async () => { + const { arrayElementChanges } = await import('../config-diff'); + expect(arrayElementChanges(['a', 'b', 'c'], ['a', 'B', 'c'])).toEqual([ + { + kind: 'changed', + beforeIndex: 1, + afterIndex: 1, + before: 'b', + after: 'B', + }, + ]); + expect(arrayElementChanges(['a', 'b', 'c'], ['x', 'a', 'b', 'c'])).toEqual([ + { kind: 'added', afterIndex: 0, after: 'x' }, + ]); + expect(arrayElementChanges(['a', 'b', 'c'], ['a', 'c'])).toEqual([ + { kind: 'removed', beforeIndex: 1, afterIndex: 1, before: 'b' }, + ]); + expect(arrayElementChanges([{ k: 1 }], [{ k: 1 }])).toEqual([]); + expect(arrayElementChanges([], [1, 2])).toHaveLength(2); + }); + + it('diffConfigs expands policy_data arrays only', () => { + const before = { + plugins: { + p: { policies: ['x'], policy_data: { users: ['a', 'b', 'c'] } }, + }, + }; + const after = { + plugins: { + p: { policies: ['x', 'y'], policy_data: { users: ['a', 'B', 'c'] } }, + }, + }; + expect(diffConfigs(before, after)).toEqual([ + { + path: '/plugins/p/policies', + kind: 'changed', + before: ['x'], + after: ['x', 'y'], + }, + { + path: '/plugins/p/policy_data/users/1', + kind: 'changed', + before: 'b', + after: 'B', + }, + ]); + // The underlying documents are untouched; a caller can opt out. + expect(diffConfigs(before, after, () => false)[1].path).toBe( + '/plugins/p/policy_data/users', + ); + }); +}); diff --git a/src/utils/agent-config/__tests__/display.spec.ts b/src/utils/agent-config/__tests__/display.spec.ts new file mode 100644 index 00000000..08b37793 --- /dev/null +++ b/src/utils/agent-config/__tests__/display.spec.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from 'vitest'; +import { formatRelative } from '../display'; + +describe('formatRelative', () => { + it('formats past and future times', () => { + const now = Date.parse('2026-09-30T12:00:00Z'); + expect(formatRelative('2026-09-30T10:00:00Z', now, 'en')).toBe( + '2 hours ago', + ); + expect(formatRelative('2026-10-03T12:00:00Z', now, 'en')).toBe('in 3 days'); + expect(formatRelative('2026-09-30T12:00:20Z', now, 'en')).toBe( + 'this minute', + ); + expect(formatRelative(null)).toBe(''); + expect(formatRelative('garbage')).toBe(''); + }); +}); diff --git a/src/utils/agent-config/__tests__/validation.spec.ts b/src/utils/agent-config/__tests__/validation.spec.ts new file mode 100644 index 00000000..bfb14f20 --- /dev/null +++ b/src/utils/agent-config/__tests__/validation.spec.ts @@ -0,0 +1,122 @@ +// Client-only checks (R89); every other rule comes from the API preview. +import { describe, expect, it } from 'vitest'; +import type { OverlayDoc } from '@/types/agent-config'; +import { + byteSize, + coerceStringMaps, + validateOverlayClientSide, +} from '../validation'; +import { parseYaml } from '../yaml'; + +function find(overlay: OverlayDoc, ptr: string) { + return validateOverlayClientSide(overlay).filter((i) => i.ptr === ptr); +} + +describe('validateOverlayClientSide (R89: client-only checks)', () => { + it('leaves the API rules to the preview', () => { + expect( + validateOverlayClientSide({ + api: null, + verbosity: 9, + plugins: { Bad: { schedule: 'nope', config: { port: '22' } } }, + } as OverlayDoc), + ).toEqual([]); + }); + + it('blocks a non-mapping overlay', () => { + expect( + validateOverlayClientSide([] as unknown as OverlayDoc)[0], + ).toMatchObject({ ptr: '', blocking: true }); + }); + + it('blocks a masked value copied from a report', () => { + const i = find( + { plugins: { ssh: { config: { password: '••••' } } } }, + '/plugins/ssh/config/password', + ); + expect(i.some((x) => x.blocking && /masked/.test(x.message))).toBe(true); + }); + + it('blocks object/array config values', () => { + const o = { + plugins: { ssh: { config: { k: { a: 1 } } } }, + } as unknown as OverlayDoc; + expect(find(o, '/plugins/ssh/config/k')[0].blocking).toBe(true); + }); + + it('only coerces booleans; every number must be quoted', () => { + const o = { + plugins: { ssh: { config: { v: 1.1, big: 2 ** 60, n: 22, on: true } } }, + } as unknown as OverlayDoc; + const c = coerceStringMaps(o); + expect(c.coerced).toEqual(['/plugins/ssh/config/on']); + const issues = validateOverlayClientSide(c.overlay); + expect( + issues + .filter((i) => i.blocking && /Quote this value/.test(i.message)) + .map((i) => i.ptr), + ).toEqual([ + '/plugins/ssh/config/v', + '/plugins/ssh/config/big', + '/plugins/ssh/config/n', + ]); + }); + + it.each(['0644', '1.0', '1e3', '0x1F', '01234'])( + 'blocks the YAML number %s instead of sending a different string', + (text) => { + const r = parseYaml(`plugins:\n ssh:\n config:\n v: ${text}\n`); + if (!r.ok) throw new Error(r.error.message); + const c = coerceStringMaps(r.value); + expect(c.coerced).toEqual([]); + expect(find(c.overlay, '/plugins/ssh/config/v')).toEqual([ + { + ptr: '/plugins/ssh/config/v', + message: expect.stringMatching(/Quote this value/), + blocking: true, + }, + ]); + }, + ); +}); + +describe('coerceStringMaps (R27)', () => { + it('converts booleans (not numbers) in config and labels and reports pointers', () => { + const o = { + plugins: { + ssh: { + config: { port: 2222, on: true, s: 'x' }, + labels: { n: 1, off: false }, + }, + }, + } as unknown as OverlayDoc; + const r = coerceStringMaps(o); + expect(r.overlay).toEqual({ + plugins: { + ssh: { + config: { port: 2222, on: 'true', s: 'x' }, + labels: { n: 1, off: 'false' }, + }, + }, + }); + expect(r.coerced).toEqual([ + '/plugins/ssh/config/on', + '/plugins/ssh/labels/off', + ]); + expect( + (o.plugins as Record }>).ssh + .config.on, + ).toBe(true); + }); + + it('returns the same object when nothing changes', () => { + const o = { plugins: { ssh: { config: { port: '22' } } } }; + expect(coerceStringMaps(o).overlay).toBe(o); + }); +}); + +describe('byteSize', () => { + it('counts UTF-8 bytes', () => { + expect(byteSize('••••')).toBe(12); + }); +}); diff --git a/src/utils/agent-config/config-diff.ts b/src/utils/agent-config/config-diff.ts new file mode 100644 index 00000000..174cc0cc --- /dev/null +++ b/src/utils/agent-config/config-diff.ts @@ -0,0 +1,237 @@ +// Client-side config diff (R16): recurses into objects; arrays and strings are compared as a +// whole, except arrays under a plugin's policy_data, which are shown element by element (an +// overlay can only replace an array whole, RFC 7396, but one edited item is one change). +// Multi-line strings are flagged so the UI can open a text diff. + +import { deepEqual, isPlainObject } from './merge-patch'; +import { escapeToken, parsePointer } from './json-pointer'; + +/** A pointer inside a plugin's policy_data (where arrays are diffed element by element). */ +export function isPolicyDataPointer(ptr: string): boolean { + const t = parsePointer(ptr); + return t.length >= 4 && t[0] === 'plugins' && t[2] === 'policy_data'; +} + +/** One element-level change between two arrays. */ +export interface ElementChange { + kind: 'added' | 'removed' | 'changed'; + /** Index in the old array (removed / changed). */ + beforeIndex?: number; + /** Index in the new array (added / changed); for a removal, where it would be re-inserted. */ + afterIndex: number; + before?: unknown; + after?: unknown; +} + +/** Above this many LCS cells, arrays are compared index by index. */ +const LCS_LIMIT = 250_000; + +/** + * Element-level changes from `before` to `after`: a longest-common-subsequence alignment (so an + * insertion does not shift every later item into a "change"), where a removal and an addition + * at the same spot pair into a `changed` element. + */ +export function arrayElementChanges( + before: readonly unknown[], + after: readonly unknown[], +): ElementChange[] { + const n = before.length; + const m = after.length; + type Step = { op: 'eq' | 'del' | 'ins'; i: number; j: number }; + const steps: Step[] = []; + if (n * m > LCS_LIMIT) { + for (let k = 0; k < Math.max(n, m); k++) { + if (k < n && k < m && deepEqual(before[k], after[k])) { + steps.push({ op: 'eq', i: k, j: k }); + continue; + } + if (k < n) steps.push({ op: 'del', i: k, j: Math.min(k, m) }); + if (k < m) steps.push({ op: 'ins', i: Math.min(k, n), j: k }); + } + } else { + // lcs[i][j]: LCS length of before[i:] and after[j:]. + const lcs = Array.from({ length: n + 1 }, () => + new Array(m + 1).fill(0), + ); + for (let i = n - 1; i >= 0; i--) { + for (let j = m - 1; j >= 0; j--) { + lcs[i][j] = deepEqual(before[i], after[j]) + ? lcs[i + 1][j + 1] + 1 + : Math.max(lcs[i + 1][j], lcs[i][j + 1]); + } + } + let i = 0; + let j = 0; + while (i < n || j < m) { + if (i < n && j < m && deepEqual(before[i], after[j])) { + steps.push({ op: 'eq', i, j }); + i++; + j++; + } else if (j < m && (i >= n || lcs[i][j + 1] >= lcs[i + 1][j])) { + steps.push({ op: 'ins', i, j }); + j++; + } else { + steps.push({ op: 'del', i, j }); + i++; + } + } + } + // Pair the deletions and insertions of each run between two equal elements. + const out: ElementChange[] = []; + let k = 0; + while (k < steps.length) { + if (steps[k].op === 'eq') { + k++; + continue; + } + const dels: Step[] = []; + const ins: Step[] = []; + while (k < steps.length && steps[k].op !== 'eq') { + (steps[k].op === 'del' ? dels : ins).push(steps[k]); + k++; + } + const paired = Math.min(dels.length, ins.length); + for (let x = 0; x < paired; x++) { + out.push({ + kind: 'changed', + beforeIndex: dels[x].i, + afterIndex: ins[x].j, + before: before[dels[x].i], + after: after[ins[x].j], + }); + } + for (const d of dels.slice(paired)) { + out.push({ + kind: 'removed', + beforeIndex: d.i, + afterIndex: d.j, + before: before[d.i], + }); + } + for (const a of ins.slice(paired)) { + out.push({ kind: 'added', afterIndex: a.j, after: after[a.j] }); + } + } + return out; +} + +/** The pointer of an element change under the array at `arrayPtr`. */ +export function elementPointer(arrayPtr: string, c: ElementChange): string { + return `${arrayPtr}/${c.kind === 'removed' ? c.beforeIndex : c.afterIndex}`; +} + +export interface DiffEntry { + path: string; + kind: 'added' | 'removed' | 'changed'; + before?: unknown; + after?: unknown; + multiline?: boolean; +} + +function isMultiline(v: unknown): boolean { + return typeof v === 'string' && v.includes('\n'); +} + +function walk( + path: string, + before: unknown, + after: unknown, + out: DiffEntry[], + expand: (ptr: string) => boolean, +): void { + if (Array.isArray(before) && Array.isArray(after) && expand(path)) { + for (const c of arrayElementChanges(before, after)) { + out.push(entry(elementPointer(path, c), c.kind, c.before, c.after)); + } + return; + } + if (isPlainObject(before) && isPlainObject(after)) { + const keys = Array.from( + new Set([...Object.keys(before), ...Object.keys(after)]), + ).sort(); + for (const key of keys) { + const p = `${path}/${escapeToken(key)}`; + const hasB = Object.prototype.hasOwnProperty.call(before, key); + const hasA = Object.prototype.hasOwnProperty.call(after, key); + if (hasB && !hasA) { + out.push(entry(p, 'removed', before[key], undefined)); + } else if (!hasB && hasA) { + out.push(entry(p, 'added', undefined, after[key])); + } else { + walk(p, before[key], after[key], out, expand); + } + } + return; + } + if (!deepEqual(before, after)) { + out.push(entry(path, 'changed', before, after)); + } +} + +function entry( + path: string, + kind: DiffEntry['kind'], + before: unknown, + after: unknown, +): DiffEntry { + const e: DiffEntry = { path, kind }; + if (kind !== 'added') e.before = before; + if (kind !== 'removed') e.after = after; + if (isMultiline(before) || isMultiline(after)) e.multiline = true; + return e; +} + +/** + * Differences between two documents. Arrays at pointers `expandArrays` accepts (by default + * those under a plugin's policy_data) are diffed element by element. + */ +export function diffConfigs( + before: unknown, + after: unknown, + expandArrays: (ptr: string) => boolean = isPolicyDataPointer, +): DiffEntry[] { + const out: DiffEntry[] = []; + walk('', before ?? {}, after ?? {}, out, expandArrays); + return out; +} + +function leafDiffPaths( + path: string, + a: unknown, + b: unknown, + hasA: boolean, + hasB: boolean, + out: string[], +): void { + let objA = isPlainObject(a) ? a : null; + let objB = isPlainObject(b) ? b : null; + if (objA && !hasB) objB = {}; + else if (objB && !hasA) objA = {}; + if (objA && objB) { + const keys = Array.from( + new Set([...Object.keys(objA), ...Object.keys(objB)]), + ).sort(); + const pushed = out.length; + for (const k of keys) { + leafDiffPaths( + `${path}/${escapeToken(k)}`, + objA[k], + objB[k], + Object.prototype.hasOwnProperty.call(objA, k), + Object.prototype.hasOwnProperty.call(objB, k), + out, + ); + } + // An object added or removed with no leaf below it (`n: {}`) is itself the change. + if (hasA !== hasB && out.length === pushed) out.push(path); + return; + } + if (hasA !== hasB || !deepEqual(a, b)) out.push(path); +} + +/** RFC 6901 pointers of every leaf that differs between two overlays (arrays are leaves). */ +export function changedLeafPaths(a: unknown, b: unknown): string[] { + const out: string[] = []; + leafDiffPaths('', a, b, true, true, out); + return out; +} diff --git a/src/utils/agent-config/display.ts b/src/utils/agent-config/display.ts new file mode 100644 index 00000000..f7c1bcbf --- /dev/null +++ b/src/utils/agent-config/display.ts @@ -0,0 +1,60 @@ +// Small display helpers for the agent Configuration tab. + +import type { ConfigDoc } from '@/types/agent-config'; +import { clone, isPlainObject } from './merge-patch'; + +/** + * Defence in depth: reports never include the client secret (the agent and the API redact + * it), but a display copy drops `api.auth.client_secret` if it is ever present. + */ +export function sanitizeForDisplay(doc: T): T { + if (!isPlainObject(doc)) return doc; + const out = clone(doc) as ConfigDoc; + if (isPlainObject(out.api) && isPlainObject(out.api.auth)) { + delete out.api.auth.client_secret; + } + return out as T; +} + +const RELATIVE_UNITS: [Intl.RelativeTimeFormatUnit, number][] = [ + ['year', 365 * 24 * 3600], + ['month', 30 * 24 * 3600], + ['week', 7 * 24 * 3600], + ['day', 24 * 3600], + ['hour', 3600], + ['minute', 60], +]; + +/** + * "2 hours ago" / "in 3 days" (Intl, no date library in the page chunk). `locale` defaults to + * the browser's. + */ +export function formatRelative( + value?: string | null, + now: number = Date.now(), + locale?: string, +): string { + if (!value) return ''; + const t = new Date(value).getTime(); + if (Number.isNaN(t)) return ''; + const seconds = Math.round((t - now) / 1000); + const rtf = new Intl.RelativeTimeFormat(locale, { numeric: 'auto' }); + for (const [unit, size] of RELATIVE_UNITS) { + if (Math.abs(seconds) >= size) + return rtf.format(Math.round(seconds / size), unit); + } + return rtf.format(0, 'minute'); +} + +export function formatAbsolute(value?: string | null): string { + if (!value) return ''; + const d = new Date(value); + if (Number.isNaN(d.getTime())) return ''; + return d.toLocaleString(); +} + +export function humanBytes(n: number): string { + if (n < 1024) return `${n} B`; + if (n < 1024 * 1024) return `${(n / 1024).toFixed(1)} KiB`; + return `${(n / (1024 * 1024)).toFixed(1)} MiB`; +} diff --git a/src/utils/agent-config/validation.ts b/src/utils/agent-config/validation.ts new file mode 100644 index 00000000..643f2607 --- /dev/null +++ b/src/utils/agent-config/validation.ts @@ -0,0 +1,152 @@ +// Client-only overlay checks (R89): what the API cannot tell the editor, or tells only after a +// round trip that would fail anyway: a mapping overlay (parsing), masked values copied from a +// report, unquoted plugin config/label scalars (R27, booleans coerced) and the plugin-name +// pattern (O6, NAME_RE: AddPluginDialog checks a name before it is part of any overlay). Every +// other rule (O1–O11) comes from the API's debounced preview (POST …/config/preview); the API +// and the agent stay authoritative. + +import { REDACTED_MASK } from '@/types/agent-config'; +import type { OverlayDoc } from '@/types/agent-config'; +import { clone, isPlainObject, type PlainObject } from './merge-patch'; +import { escapeToken, pointer } from './json-pointer'; + +export interface ClientIssue { + ptr: string; + message: string; + blocking: boolean; +} + +/** + * Plugin names (R28, rule O6). An intentional client-side copy: it must match the API's + * `PluginNamePattern` exactly. + */ +export const NAME_RE = /^[a-z0-9][a-z0-9_-]{0,62}$/; + +export const LIMITS = { + /** Larger drafts are checked on Review only (live preview). */ + overlayBytes: 256 * 1024, + commentChars: 2000, +} as const; + +const encoder = new TextEncoder(); + +export function byteSize(str: string): number { + return encoder.encode(str).length; +} + +/** + * R27: plugin config and label values are strings. Converts BOOLEAN values of + * `plugins.*.config` / `labels` to "true" / "false" (no information is lost). Numbers are left + * for the blocking "Quote this value" check: YAML has already lost their source text (`0644`, + * `1.0`, `1e3`, `0x1F` all load as numbers whose String() differs from what was typed). + * Objects and arrays are left for the (blocking) validation too. + */ +export function coerceStringMaps(overlay: OverlayDoc): { + overlay: OverlayDoc; + coerced: string[]; +} { + const coerced: string[] = []; + const plugins = overlay.plugins; + if (!isPlainObject(plugins)) return { overlay, coerced }; + let out: OverlayDoc | null = null; + for (const [name, plugin] of Object.entries(plugins)) { + if (!isPlainObject(plugin)) continue; + for (const field of ['config', 'labels'] as const) { + const map = plugin[field]; + if (!isPlainObject(map)) continue; + for (const [k, v] of Object.entries(map)) { + if (typeof v === 'boolean') { + if (!out) out = clone(overlay); + ( + ((out.plugins as PlainObject)[name] as PlainObject)[ + field + ] as PlainObject + )[k] = String(v); + coerced.push(pointer('plugins', name, field, k)); + } + } + } + } + return { overlay: out ?? overlay, coerced }; +} + +function walkStrings( + ptr: string, + value: unknown, + fn: (ptr: string, s: string) => void, +): void { + if (typeof value === 'string') { + fn(ptr, value); + } else if (Array.isArray(value)) { + value.forEach((v, i) => walkStrings(`${ptr}/${i}`, v, fn)); + } else if (isPlainObject(value)) { + for (const [k, v] of Object.entries(value)) { + walkStrings(`${ptr}/${escapeToken(k)}`, v, fn); + } + } +} + +/** + * The client-only checks of an overlay (R89). Blocking issues disable Review & save and the + * live preview. + */ +export function validateOverlayClientSide(overlay: OverlayDoc): ClientIssue[] { + const issues: ClientIssue[] = []; + const seen = new Set(); + const add = (ptr: string, message: string, blocking: boolean) => { + const key = `${ptr}\u0000${message}`; + if (seen.has(key)) return; + seen.add(key); + issues.push({ ptr, message, blocking }); + }; + if (!isPlainObject(overlay)) { + add('', 'Overlay must be a mapping', true); + return issues; + } + + // R27: plugin config and label values are strings. + const plugins = overlay.plugins; + if (isPlainObject(plugins)) { + for (const [name, plugin] of Object.entries(plugins)) { + if (!isPlainObject(plugin)) continue; + for (const field of ['config', 'labels'] as const) { + const map = plugin[field]; + if (!isPlainObject(map)) continue; + for (const [k, v] of Object.entries(map)) { + const kptr = pointer('plugins', name, field, k); + if (isPlainObject(v) || Array.isArray(v)) { + add(kptr, 'Values must be strings', true); + } else if (typeof v === 'number' || typeof v === 'boolean') { + // Only booleans are converted (R27); YAML already lost the original text of a number + // (0644, 1.0, 1e3, 0x1F), so ask for quotes instead of guessing. + add( + kptr, + 'Quote this value: plugin config and label values are strings', + true, + ); + } + } + } + } + } + + // R25: a masked value copied from a report would be saved literally. + for (const ptr of maskedPointers(overlay)) { + add(ptr, 'This looks like a masked value copied from a report', true); + } + + return issues; +} + +/** Pointers of every string value equal to the report mask ("••••", R25). */ +export function maskedPointers(doc: unknown): string[] { + const out: string[] = []; + walkStrings('', doc, (ptr, s) => { + if (s === REDACTED_MASK) out.push(ptr); + }); + return out; +} + +export function hasBlocking(issues: ClientIssue[]): boolean { + return issues.some((i) => i.blocking); +} From 86414936e6eb9c231b06cdcb791b067fe8267c19 Mon Sep 17 00:00:00 2001 From: "ccf-lisa[bot]" <286799724+ccf-lisa[bot]@users.noreply.github.com> Date: Tue, 6 Oct 2026 06:27:59 -0300 Subject: [PATCH 2/2] fix(agent-config): block unstorable numbers; align large arrays by their common ends - validateOverlayClientSide blocks Infinity, NaN and integers past 2^53 anywhere in the overlay: JSON.stringify writes non-finite numbers as null, which RFC 7396 reads as deleting the key on every host - arrayElementChanges matches the common prefix and suffix before the LCS, so one insertion in an array past the LCS limit is still one addition (regression test at 600 x 601) - sanitizeForDisplay spec: client_secret removed from the copy only - plugin-name cases join the agentconfig conformance table Co-Authored-By: Claude Opus 5.5 --- .../__tests__/agentconfig-conformance.spec.ts | 10 +++ .../__tests__/config-diff.spec.ts | 27 +++++++ .../agent-config/__tests__/display.spec.ts | 28 +++++++- .../agent-config/__tests__/validation.spec.ts | 28 +++++++- src/utils/agent-config/config-diff.ts | 71 ++++++++++++------- src/utils/agent-config/validation.ts | 43 +++++++++-- 6 files changed, 176 insertions(+), 31 deletions(-) diff --git a/src/utils/agent-config/__tests__/agentconfig-conformance.spec.ts b/src/utils/agent-config/__tests__/agentconfig-conformance.spec.ts index ece52ec9..7b9a8628 100644 --- a/src/utils/agent-config/__tests__/agentconfig-conformance.spec.ts +++ b/src/utils/agent-config/__tests__/agentconfig-conformance.spec.ts @@ -12,6 +12,7 @@ import fixture from './fixtures/agentconfig-conformance.json'; import { configKeyOverridable, sourceTrusted } from '../glob'; import { validateCron5 } from '../cron5'; import { fieldAccess, sourceKind } from '../field-access'; +import { NAME_RE } from '../validation'; interface Conformance { trustedSources: { @@ -28,6 +29,7 @@ interface Conformance { }[]; sourceKinds: [string, 'oci' | 'local'][]; schedules: { valid: string[]; invalid: string[] }; + pluginNames: { valid: string[]; invalid: string[] }; applySafe: { cases: { name: string; @@ -116,6 +118,14 @@ describe('pkg/agentconfig conformance', () => { expect(validateCron5(expr)).not.toBeNull(); }); + it.each(cases.pluginNames.valid)('PluginNamePattern accepts %j', (name) => { + expect(NAME_RE.test(name)).toBe(true); + }); + + it.each(cases.pluginNames.invalid)('PluginNamePattern rejects %j', (name) => { + expect(NAME_RE.test(name)).toBe(false); + }); + it.each(cases.applySafe.cases)( 'apply_safe: $name', ({ trusted, file, overlay, path, state }) => { diff --git a/src/utils/agent-config/__tests__/config-diff.spec.ts b/src/utils/agent-config/__tests__/config-diff.spec.ts index bf1660bb..3cf8771e 100644 --- a/src/utils/agent-config/__tests__/config-diff.spec.ts +++ b/src/utils/agent-config/__tests__/config-diff.spec.ts @@ -86,6 +86,33 @@ describe('element-level array changes', () => { expect(arrayElementChanges([], [1, 2])).toHaveLength(2); }); + it('keeps insertion alignment for arrays past the LCS limit', async () => { + const { arrayElementChanges } = await import('../config-diff'); + const before = Array.from({ length: 600 }, (_, i) => `item-${i}`); + // 600 × 601 cells is past the limit: one insertion is still one addition. + expect(arrayElementChanges(before, ['new', ...before])).toEqual([ + { kind: 'added', afterIndex: 0, after: 'new' }, + ]); + const inserted = [...before.slice(0, 3), 'new', ...before.slice(3)]; + expect(arrayElementChanges(before, inserted)).toEqual([ + { kind: 'added', afterIndex: 3, after: 'new' }, + ]); + const removed = before.filter((_, i) => i !== 10); + expect(arrayElementChanges(before, removed)).toEqual([ + { kind: 'removed', beforeIndex: 10, afterIndex: 10, before: 'item-10' }, + ]); + const edited = before.map((v, i) => (i === 300 ? 'changed' : v)); + expect(arrayElementChanges(before, edited)).toEqual([ + { + kind: 'changed', + beforeIndex: 300, + afterIndex: 300, + before: 'item-300', + after: 'changed', + }, + ]); + }); + it('diffConfigs expands policy_data arrays only', () => { const before = { plugins: { diff --git a/src/utils/agent-config/__tests__/display.spec.ts b/src/utils/agent-config/__tests__/display.spec.ts index 08b37793..65a5c4c9 100644 --- a/src/utils/agent-config/__tests__/display.spec.ts +++ b/src/utils/agent-config/__tests__/display.spec.ts @@ -1,5 +1,31 @@ import { describe, expect, it } from 'vitest'; -import { formatRelative } from '../display'; +import { formatRelative, sanitizeForDisplay } from '../display'; + +describe('sanitizeForDisplay', () => { + it('drops api.auth.client_secret from a copy and leaves the input alone', () => { + const doc = { + api: { + url: 'https://api.example.com', + auth: { client_id: 'id', client_secret: 's3cret' }, + }, + plugins: { ssh: { config: { client_secret: 'kept: not the API key' } } }, + }; + const input = structuredClone(doc); + const out = sanitizeForDisplay(doc); + expect(out).toEqual({ + api: { url: 'https://api.example.com', auth: { client_id: 'id' } }, + plugins: { ssh: { config: { client_secret: 'kept: not the API key' } } }, + }); + expect(doc).toEqual(input); + expect(out).not.toBe(doc); + }); + + it('passes through documents without api.auth and non-objects', () => { + expect(sanitizeForDisplay({ verbosity: 1 })).toEqual({ verbosity: 1 }); + expect(sanitizeForDisplay(null)).toBeNull(); + expect(sanitizeForDisplay('x')).toBe('x'); + }); +}); describe('formatRelative', () => { it('formats past and future times', () => { diff --git a/src/utils/agent-config/__tests__/validation.spec.ts b/src/utils/agent-config/__tests__/validation.spec.ts index bfb14f20..6b9db3a2 100644 --- a/src/utils/agent-config/__tests__/validation.spec.ts +++ b/src/utils/agent-config/__tests__/validation.spec.ts @@ -62,7 +62,7 @@ describe('validateOverlayClientSide (R89: client-only checks)', () => { ]); }); - it.each(['0644', '1.0', '1e3', '0x1F', '01234'])( + it.each(['1.0', '1e3'])( 'blocks the YAML number %s instead of sending a different string', (text) => { const r = parseYaml(`plugins:\n ssh:\n config:\n v: ${text}\n`); @@ -78,6 +78,32 @@ describe('validateOverlayClientSide (R89: client-only checks)', () => { ]); }, ); + + it.each(['0644', '0x1F', '01234'])( + 'the YAML parser already rejects the ambiguous number %s', + (text) => { + expect( + parseYaml(`plugins:\n ssh:\n config:\n v: ${text}\n`).ok, + ).toBe(false); + }, + ); + + it('blocks numbers JSON cannot carry as typed, in every field', () => { + // Raw JSON views: JSON.parse gives Infinity for 1e999 and rounds 20-digit integers. + const o = JSON.parse( + '{"verbosity": 1e999, "plugins": {"p": {"policy_data": {"limit": 1e999, "id": 12345678901234567890, "ok": [1, 2.5, -0, 9007199254740991]}}}}', + ) as OverlayDoc; + (o.plugins!.p!.policy_data as Record).nan = NaN; + const ptrs = validateOverlayClientSide(o) + .filter((i) => i.blocking && /cannot be saved as typed/.test(i.message)) + .map((i) => i.ptr); + expect(ptrs).toEqual([ + '/verbosity', + '/plugins/p/policy_data/limit', + '/plugins/p/policy_data/id', + '/plugins/p/policy_data/nan', + ]); + }); }); describe('coerceStringMaps (R27)', () => { diff --git a/src/utils/agent-config/config-diff.ts b/src/utils/agent-config/config-diff.ts index 174cc0cc..11ae22d6 100644 --- a/src/utils/agent-config/config-diff.ts +++ b/src/utils/agent-config/config-diff.ts @@ -23,13 +23,15 @@ export interface ElementChange { after?: unknown; } -/** Above this many LCS cells, arrays are compared index by index. */ +/** Above this many LCS cells (after trimming the common ends), the rest is compared index by index. */ const LCS_LIMIT = 250_000; /** * Element-level changes from `before` to `after`: a longest-common-subsequence alignment (so an * insertion does not shift every later item into a "change"), where a removal and an addition - * at the same spot pair into a `changed` element. + * at the same spot pair into a `changed` element. The common prefix and suffix are matched + * first, so one edit in an array of any size (the usual case) aligns exactly in linear memory; + * only a middle larger than LCS_LIMIT cells falls back to index by index. */ export function arrayElementChanges( before: readonly unknown[], @@ -39,40 +41,59 @@ export function arrayElementChanges( const m = after.length; type Step = { op: 'eq' | 'del' | 'ins'; i: number; j: number }; const steps: Step[] = []; - if (n * m > LCS_LIMIT) { - for (let k = 0; k < Math.max(n, m); k++) { - if (k < n && k < m && deepEqual(before[k], after[k])) { - steps.push({ op: 'eq', i: k, j: k }); + let pre = 0; + while (pre < n && pre < m && deepEqual(before[pre], after[pre])) pre++; + let suf = 0; + while ( + suf < n - pre && + suf < m - pre && + deepEqual(before[n - 1 - suf], after[m - 1 - suf]) + ) { + suf++; + } + // The middle still to align: before[pre, ne) and after[pre, me). + const ne = n - suf; + const me = m - suf; + const bn = ne - pre; + const bm = me - pre; + if (bn * bm > LCS_LIMIT) { + for (let k = 0; k < Math.max(bn, bm); k++) { + const i = pre + k; + const j = pre + k; + if (i < ne && j < me && deepEqual(before[i], after[j])) { + steps.push({ op: 'eq', i, j }); continue; } - if (k < n) steps.push({ op: 'del', i: k, j: Math.min(k, m) }); - if (k < m) steps.push({ op: 'ins', i: Math.min(k, n), j: k }); + if (i < ne) steps.push({ op: 'del', i, j: Math.min(j, me) }); + if (j < me) steps.push({ op: 'ins', i: Math.min(i, ne), j }); } } else { - // lcs[i][j]: LCS length of before[i:] and after[j:]. - const lcs = Array.from({ length: n + 1 }, () => - new Array(m + 1).fill(0), + // lcs[a][b]: LCS length of before[pre + a, ne) and after[pre + b, me). + const lcs = Array.from({ length: bn + 1 }, () => + new Array(bm + 1).fill(0), ); - for (let i = n - 1; i >= 0; i--) { - for (let j = m - 1; j >= 0; j--) { - lcs[i][j] = deepEqual(before[i], after[j]) - ? lcs[i + 1][j + 1] + 1 - : Math.max(lcs[i + 1][j], lcs[i][j + 1]); + for (let a = bn - 1; a >= 0; a--) { + for (let b = bm - 1; b >= 0; b--) { + lcs[a][b] = deepEqual(before[pre + a], after[pre + b]) + ? lcs[a + 1][b + 1] + 1 + : Math.max(lcs[a + 1][b], lcs[a][b + 1]); } } - let i = 0; - let j = 0; - while (i < n || j < m) { - if (i < n && j < m && deepEqual(before[i], after[j])) { + let a = 0; + let b = 0; + while (a < bn || b < bm) { + const i = pre + a; + const j = pre + b; + if (a < bn && b < bm && deepEqual(before[i], after[j])) { steps.push({ op: 'eq', i, j }); - i++; - j++; - } else if (j < m && (i >= n || lcs[i][j + 1] >= lcs[i + 1][j])) { + a++; + b++; + } else if (b < bm && (a >= bn || lcs[a][b + 1] >= lcs[a + 1][b])) { steps.push({ op: 'ins', i, j }); - j++; + b++; } else { steps.push({ op: 'del', i, j }); - i++; + a++; } } } diff --git a/src/utils/agent-config/validation.ts b/src/utils/agent-config/validation.ts index 643f2607..78cb3a98 100644 --- a/src/utils/agent-config/validation.ts +++ b/src/utils/agent-config/validation.ts @@ -1,7 +1,8 @@ // Client-only overlay checks (R89): what the API cannot tell the editor, or tells only after a // round trip that would fail anyway: a mapping overlay (parsing), masked values copied from a // report, unquoted plugin config/label scalars (R27, booleans coerced) and the plugin-name -// pattern (O6, NAME_RE: AddPluginDialog checks a name before it is part of any overlay). Every +// pattern (O6, NAME_RE: AddPluginDialog checks a name before it is part of any overlay), and +// numbers JSON cannot carry as typed (Infinity / NaN become null, a delete). Every // other rule (O1–O11) comes from the API's debounced preview (POST …/config/preview); the API // and the agent stay authoritative. @@ -37,9 +38,8 @@ export function byteSize(str: string): number { /** * R27: plugin config and label values are strings. Converts BOOLEAN values of * `plugins.*.config` / `labels` to "true" / "false" (no information is lost). Numbers are left - * for the blocking "Quote this value" check: YAML has already lost their source text (`0644`, - * `1.0`, `1e3`, `0x1F` all load as numbers whose String() differs from what was typed). - * Objects and arrays are left for the (blocking) validation too. + * for the blocking "Quote this value" check: `1.0` and `1e3` load as numbers whose String() + * differs from what was typed. Objects and arrays are left for the (blocking) validation too. */ export function coerceStringMaps(overlay: OverlayDoc): { overlay: OverlayDoc; @@ -70,6 +70,31 @@ export function coerceStringMaps(overlay: OverlayDoc): { return { overlay: out ?? overlay, coerced }; } +/** True when `n` survives a JSON round trip as typed: finite, and an integer only up to 2^53. */ +export function isStorableNumber(n: number): boolean { + return ( + Number.isFinite(n) && + !(Number.isInteger(n) && Math.abs(n) > Number.MAX_SAFE_INTEGER) + ); +} + +/** Pointers of every number in `doc` that would not be stored as typed (isStorableNumber). */ +export function unstorableNumberPointers(doc: unknown): string[] { + const out: string[] = []; + const walk = (ptr: string, v: unknown) => { + if (typeof v === 'number') { + if (!isStorableNumber(v)) out.push(ptr); + } else if (Array.isArray(v)) { + v.forEach((x, i) => walk(`${ptr}/${i}`, x)); + } else if (isPlainObject(v)) { + for (const [k, x] of Object.entries(v)) + walk(`${ptr}/${escapeToken(k)}`, x); + } + }; + walk('', doc); + return out; +} + function walkStrings( ptr: string, value: unknown, @@ -130,6 +155,16 @@ export function validateOverlayClientSide(overlay: OverlayDoc): ClientIssue[] { } } + // JSON.stringify writes Infinity / NaN as null, which RFC 7396 reads as a delete of that key + // on every host, and integers above 2^53 lose digits. + for (const ptr of unstorableNumberPointers(overlay)) { + add( + ptr, + 'This number cannot be saved as typed (not finite, or more digits than 2^53); quote it to keep it as text', + true, + ); + } + // R25: a masked value copied from a report would be saved literally. for (const ptr of maskedPointers(overlay)) { add(ptr, 'This looks like a masked value copied from a report', true);