From bd65c9dccf3d9c9352254b38dcd1b7ff47f3809b Mon Sep 17 00:00:00 2001 From: AlexRV12 <71396855+AlexRV12@users.noreply.github.com> Date: Wed, 26 Aug 2026 11:07:49 +0200 Subject: [PATCH] fix: redact secrets under a oneOf array element, and disable a dynamic input on request Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/lib/components/ArgInput.svelte | 1 + frontend/src/lib/components/job_args.test.ts | 39 +++++++ frontend/src/lib/components/job_args.ts | 104 ++++++++++++------- 3 files changed, 108 insertions(+), 36 deletions(-) diff --git a/frontend/src/lib/components/ArgInput.svelte b/frontend/src/lib/components/ArgInput.svelte index 8550a58b57..832f5e4e6c 100644 --- a/frontend/src/lib/components/ArgInput.svelte +++ b/frontend/src/lib/components/ArgInput.svelte @@ -1078,6 +1078,7 @@ {otherArgs} {helperScript} {workspace} + {disabled} bind:value format={format ?? ''} /> diff --git a/frontend/src/lib/components/job_args.test.ts b/frontend/src/lib/components/job_args.test.ts index d036f0d30e..e96eaf4e8d 100644 --- a/frontend/src/lib/components/job_args.test.ts +++ b/frontend/src/lib/components/job_args.test.ts @@ -359,6 +359,24 @@ describe('enforceDisabledDefaults', () => { const { args } = enforceDisabledDefaults({ either: { kind: 'a', level: 99 } }, schema) expect(args.either).toEqual({ kind: 'a', level: 1 }) }) + + // `ArgInput` settles on the first tag that titles a branch, so a tag titling none + // leaves it on the branch the other tag names. Stopping at the first tag holding a + // string resolved to branch 0 instead, and enforced its default over that branch. + it('reads past a oneOf tag that names no branch', () => { + const tagged = { + properties: { + either: { + oneOf: [ + { title: 'a', properties: { level: { type: 'number', disabled: true, default: 1 } } }, + { title: 'b', properties: { level: { type: 'number' } } } + ] + } + } + } + const value = { either: { kind: 'nosuchbranch', label: 'b', level: 99 } } + expect(enforceDisabledDefaults(value, tagged).args.either).toEqual(value.either) + }) }) describe('secret args at every level the form nests', () => { @@ -404,6 +422,27 @@ describe('secret args at every level the form nests', () => { expect(Object.keys(stripSecretArgs({ top: 'x' }, schema))).toEqual([]) }) + // Array elements were descended only through `items.properties`, so a branch under + // `items.oneOf` was kept by conformArgsToSchema and then never visited: the secret + // reached the persisted card and the model-visible result verbatim. + it('strips a secret under a oneOf branch of an array element', () => { + const oneOfItems = { + properties: { + steps: { + type: 'array', + items: { + oneOf: [{ title: 'push', properties: { token: { type: 'string', password: true } } }] + } + } + } + } + const stripped: string[] = [] + expect( + stripSecretArgs({ steps: [{ token: 'hunter2', name: 'a' }] }, oneOfItems, stripped) + ).toEqual({ steps: [{ name: 'a' }] }) + expect(stripped).toEqual(['steps[0].token']) + }) + // The caller binds the result to a form that edits in place, so a schema declaring // nothing must not hand back the object it was given. it('copies even when the schema declares nothing to strip', () => { diff --git a/frontend/src/lib/components/job_args.ts b/frontend/src/lib/components/job_args.ts index 363ee2b066..bde36db3ff 100644 --- a/frontend/src/lib/components/job_args.ts +++ b/frontend/src/lib/components/job_args.ts @@ -209,10 +209,15 @@ function unionDeclaredProperties(bags: Record[], value: any): Recor } /** The `oneOf` branch the form shows: `ArgInput` opens the one the value's tag names, - * and the first branch when no tag names any. */ + * and the first branch when no tag names any. The first tag that names a branch, not the + * first that holds a string — a tag naming nothing leaves `ArgInput` on the branch the + * other tag names, so stopping at it would enforce a branch the form never opened. */ function selectedOneOfBranch(value: any, oneOf: any[]): any { - const tag = ONE_OF_TAG_KEYS.map((key) => value?.[key]).find((v) => typeof v === 'string') - return oneOf.find((branch) => tag != null && branch?.title === tag) ?? oneOf[0] + for (const key of ONE_OF_TAG_KEYS) { + const named = oneOf.find((branch) => branch?.title === value?.[key]) + if (named) return named + } + return oneOf[0] } /** @@ -257,21 +262,11 @@ function mapMatchingArgs( // Absent means the form never carried this level; recursing would rebuild it and // leave the key behind holding undefined. if (!Object.hasOwn(result, key)) continue - if (prop?.properties) { - result[key] = mapMatchingArgs( - result[key], - prop.properties, - isLeaf, - visit, - selectedBranchOnly, - keyPath, - inOneOf - ) - } else if (prop?.items?.properties && Array.isArray(result[key])) { + if (prop?.items && !prop?.properties && Array.isArray(result[key])) { result[key] = result[key].map((item: any, i: number) => - mapMatchingArgs( + mapMatchingNested( item, - prop.items.properties, + prop.items, isLeaf, visit, selectedBranchOnly, @@ -279,32 +274,69 @@ function mapMatchingArgs( inOneOf ) ) - } else if (Array.isArray(prop?.oneOf)) { - // One branch or all of them, and the difference is the visitor: a value can carry - // a key from a variant nobody selected, so stripping has to reach every branch, - // while a default read off an unselected branch is one the form never showed. - const branches = selectedBranchOnly - ? [selectedOneOfBranch(result[key], prop.oneOf)] - : prop.oneOf - for (const branch of branches) { - if (branch?.properties) { - result[key] = mapMatchingArgs( - result[key], - branch.properties, - isLeaf, - visit, - selectedBranchOnly, - keyPath, - !selectedBranchOnly - ) - } - } + } else { + result[key] = mapMatchingNested( + result[key], + prop, + isLeaf, + visit, + selectedBranchOnly, + keyPath, + inOneOf + ) } } // Spread rather than the accumulator itself, so callers get a plain object back. return { ...result } } +/** + * Descend one level, whether the declaration nests its fields in `properties` or spreads + * them across `oneOf` branches. Shared with the array case: an element is declared either + * way, and a password reached down one shape has to be reached down the other. + */ +function mapMatchingNested( + holder: any, + prop: any, + isLeaf: (prop: any) => boolean, + visit: (value: unknown, prop: any, path: string) => unknown, + selectedBranchOnly: boolean, + path: string, + inOneOf: boolean +): any { + if (prop?.properties) { + return mapMatchingArgs( + holder, + prop.properties, + isLeaf, + visit, + selectedBranchOnly, + path, + inOneOf + ) + } + if (!Array.isArray(prop?.oneOf)) return holder + // One branch or all of them, and the difference is the visitor: a value can carry a key + // from a variant nobody selected, so stripping has to reach every branch, while a + // default read off an unselected branch is one the form never showed. + const branches = selectedBranchOnly ? [selectedOneOfBranch(holder, prop.oneOf)] : prop.oneOf + let mapped = holder + for (const branch of branches) { + if (branch?.properties) { + mapped = mapMatchingArgs( + mapped, + branch.properties, + isLeaf, + visit, + selectedBranchOnly, + path, + !selectedBranchOnly + ) + } + } + return mapped +} + const isSecretProp = (prop: any) => !!prop?.password const isFileProp = (prop: any) =>