mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 08:01:35 +00:00
fix: let a variable reference fill a secret run-form argument
This commit is contained in:
@@ -4930,7 +4930,7 @@ describe('global AI tools', () => {
|
||||
|
||||
// A secret the model picked is not consent, and a result that echoed one back would let
|
||||
// it propose the same value again on the next call.
|
||||
it('run_script opens password fields empty and hides secrets from the model', async () => {
|
||||
it('run_script empties a proposed secret but keeps a variable reference', async () => {
|
||||
vi.mocked(ScriptService.getScriptByPath).mockResolvedValueOnce({
|
||||
path: 'f/scripts/rotate',
|
||||
schema: {
|
||||
@@ -4969,10 +4969,13 @@ describe('global AI tools', () => {
|
||||
)
|
||||
)
|
||||
|
||||
expect(shown).toEqual({ nested: {}, name: 'ada' })
|
||||
// The literal is emptied; the variable reference is the model's to send and survives,
|
||||
// since the secret stays in the variable and only its path travels.
|
||||
expect(shown).toEqual({ nested: { inner: '$var:u/ada/prod_api_key' }, name: 'ada' })
|
||||
// Named, or an emptied field reads as the user having deleted the value and the
|
||||
// next call proposes the same secret again.
|
||||
expect(result).toContain('token, nested.inner')
|
||||
expect(result).toContain('token')
|
||||
expect(result).not.toContain('nested.inner')
|
||||
expect(result).not.toContain('hunter2')
|
||||
expect(result).not.toContain('secret_arg')
|
||||
expect(result).not.toContain('prod_api_key')
|
||||
|
||||
@@ -164,14 +164,27 @@ describe('secret args at every level the form nests', () => {
|
||||
either: { kind: 'a', key: 'k', other: 'o' }
|
||||
}
|
||||
|
||||
it('strips every one of them', () => {
|
||||
it('strips every one of them, keeping a variable reference', () => {
|
||||
expect(stripSecretArgs(args, schema)).toEqual({
|
||||
obj: { keep: 1 },
|
||||
// Naming a workspace variable is how a secret is meant to reach a job, and the
|
||||
// value never leaves it — so the reference is the caller's to send.
|
||||
obj: { inner: '$var:u/ada/prod', keep: 1 },
|
||||
list: [{ name: 'a' }, {}],
|
||||
either: { kind: 'a' }
|
||||
})
|
||||
})
|
||||
|
||||
// `$jsonvar:` paths are minted from what the user typed into the form, so a caller naming
|
||||
// one is naming a secret it was never shown. A bare `$var:` names nothing.
|
||||
it('keeps only a reference that names a workspace variable', () => {
|
||||
const one = { properties: { token: { type: 'string', password: true } } }
|
||||
const kept = (v: unknown) => stripSecretArgs({ token: v }, one).token
|
||||
expect(kept('$var:f/team/api_token')).toBe('$var:f/team/api_token')
|
||||
expect(kept('$jsonvar:u/ada/secret_arg/abc123')).toBeUndefined()
|
||||
expect(kept('$var:')).toBeUndefined()
|
||||
expect(kept('hunter2')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('redacts every one of them', () => {
|
||||
const redacted = JSON.stringify(redactSecretArgs(args, schema))
|
||||
for (const secret of ['hunter2', 'prod', 'one', 'two', '"k"', '"o"']) {
|
||||
|
||||
@@ -186,6 +186,13 @@ function mapLeaves(
|
||||
|
||||
const isSecretProp = (prop: any) => !!prop?.password
|
||||
|
||||
/** A reference to a workspace variable, which is how a secret is meant to reach a job: the
|
||||
* value stays in the variable and the argument carries only its path, so it is safe to show
|
||||
* and safe to store. `$jsonvar:` is deliberately not one of these — those are minted from
|
||||
* what the user typed, so a caller naming one is naming a secret it was never shown. */
|
||||
const isVariableRef = (value: unknown) =>
|
||||
typeof value === 'string' && /^\$var:\S/.test(value)
|
||||
|
||||
const isFileProp = (prop: any) =>
|
||||
prop?.contentEncoding === 'base64' || prop?.items?.contentEncoding === 'base64'
|
||||
|
||||
@@ -209,11 +216,15 @@ function mapArgLeaves(
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop every password-typed argument, so a caller cannot propose a secret on the user's
|
||||
* behalf: the field falls back to whatever the script itself declares, as on any other
|
||||
* run form, and the user fills in the rest. Appends the path of
|
||||
* each one removed, so the caller can be told the field was emptied rather than left to
|
||||
* read the absence as the user having deleted it.
|
||||
* Drop every password-typed argument holding a secret of its own, so a caller cannot propose
|
||||
* one on the user's behalf: the field falls back to whatever the script itself declares, as on
|
||||
* any other run form, and the user fills in the rest. Appends the path of each one removed, so
|
||||
* the caller can be told the field was emptied rather than left to read the absence as the user
|
||||
* having deleted it.
|
||||
*
|
||||
* A `$var:` reference is kept: naming a workspace variable is how a secret is meant to reach a
|
||||
* job, the caller can list those already, and the field's own widget treats an incoming
|
||||
* reference as filled rather than minting over it.
|
||||
*/
|
||||
export function stripSecretArgs(
|
||||
args: Record<string, any>,
|
||||
@@ -221,6 +232,7 @@ export function stripSecretArgs(
|
||||
strippedKeys?: string[]
|
||||
): Record<string, any> {
|
||||
return mapArgLeaves(args, schema, isSecretProp, (value, _prop, path) => {
|
||||
if (isVariableRef(value)) return value
|
||||
if (value !== undefined) strippedKeys?.push(path)
|
||||
return undefined
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user