From 64d2fad4c67c85f5bde426c11f9999fd11da21da Mon Sep 17 00:00:00 2001 From: keeb Date: Sat, 28 Mar 2026 10:31:25 -0700 Subject: [PATCH] fix: support spaces in quoted vault.get() arguments The vault.get() regex in model_resolver.ts used character classes that excluded whitespace even inside quoted arguments. This broke vault expressions referencing secrets with spaces in their names, such as 1Password fields like "Client ID" or paths like "Tailscale K8s Operator/Client ID". Update the regex to use alternation: quoted arguments now match any character except the closing quote, while unquoted arguments preserve current no-space behavior. Update match extraction for the new capture group positions. Fixes #902 Co-authored-by: Dan McClain --- .../expression_evaluation_service_test.ts | 13 +++ src/domain/expressions/model_resolver.ts | 16 +++- src/domain/vaults/vault_expression_test.ts | 94 +++++++++++++++++++ 3 files changed, 119 insertions(+), 4 deletions(-) diff --git a/src/domain/expressions/expression_evaluation_service_test.ts b/src/domain/expressions/expression_evaluation_service_test.ts index c09337e4..d522972a 100644 --- a/src/domain/expressions/expression_evaluation_service_test.ts +++ b/src/domain/expressions/expression_evaluation_service_test.ts @@ -77,6 +77,19 @@ Deno.test("containsVaultExpression returns false for vault-like but not vault.ge assertEquals(containsVaultExpression("vault_get(foo)"), false); }); +Deno.test("containsVaultExpression returns true for quoted args with spaces", () => { + assertEquals( + containsVaultExpression('vault.get("infra", "Client ID")'), + true, + ); + assertEquals( + containsVaultExpression( + 'vault.get("infra", "Tailscale K8s Operator/Client ID")', + ), + true, + ); +}); + // ============================================================================ // containsEnvExpression // ============================================================================ diff --git a/src/domain/expressions/model_resolver.ts b/src/domain/expressions/model_resolver.ts index 2722a8a1..5b450ec0 100644 --- a/src/domain/expressions/model_resolver.ts +++ b/src/domain/expressions/model_resolver.ts @@ -825,10 +825,14 @@ export class ModelResolver { redactor?: SecretRedactor, secretBag?: VaultSecretBag, ): Promise { - // Pattern to match vault.get(vaultName, secretKey) expressions - // Handles both quoted and unquoted arguments + // Pattern to match vault.get(vaultName, secretKey) expressions. + // Handles both quoted and unquoted arguments. Quoted arguments may + // contain spaces (e.g. vault.get("infra", "Client ID")). + // Each argument uses alternation: + // (['"`])(.+?)\1 — quoted: any chars up to the matching close quote + // ([^\s,)]+) — unquoted: non-whitespace, non-comma, non-paren const vaultPattern = - /vault\.get\(\s*(['"`]?)([^'"`\s,]+)\1\s*,\s*(['"`]?)([^'"`\s,]+)\3\s*\)/g; + /vault\.get\(\s*(?:(['"`])(.+?)\1|([^\s,)]+))\s*,\s*(?:(['"`])(.+?)\4|([^\s,)]+))\s*\)/g; let resolvedValue = value; const matches = Array.from(value.matchAll(vaultPattern)); @@ -841,7 +845,11 @@ export class ModelResolver { const vaultService = await this.getVaultService(); for (const match of matches) { - const [fullMatch, , vaultName, , secretKey] = match; + // Groups: [1]=quote1, [2]=quoted vault, [3]=unquoted vault, + // [4]=quote2, [5]=quoted key, [6]=unquoted key + const fullMatch = match[0]; + const vaultName = match[2] ?? match[3]; + const secretKey = match[5] ?? match[6]; try { const secretValue = await vaultService.get(vaultName, secretKey); redactor?.addSecret(secretValue); diff --git a/src/domain/vaults/vault_expression_test.ts b/src/domain/vaults/vault_expression_test.ts index f6a5885f..720fc7e1 100644 --- a/src/domain/vaults/vault_expression_test.ts +++ b/src/domain/vaults/vault_expression_test.ts @@ -93,6 +93,18 @@ Deno.test("ModelResolver.resolveVaultExpressions", async (t) => { return new ModelResolver(definitionRepo, { vaultService }); } + // Helper to create a ModelResolver with multiple named vaults + function createResolverWithNamedVaults( + vaults: Record>, + ): ModelResolver { + const vaultService = new VaultService(); + for (const [name, secrets] of Object.entries(vaults)) { + vaultService.registerVault({ name, type: "mock", config: secrets }); + } + const definitionRepo = new YamlDefinitionRepository(tempDir); + return new ModelResolver(definitionRepo, { vaultService }); + } + await t.step("should resolve basic vault expression", async () => { const resolver = createResolverWithMockVault({ "api-key": "secret123" }); const result = await resolver.resolveVaultExpressions( @@ -365,6 +377,88 @@ Deno.test("ModelResolver.resolveVaultExpressions", async (t) => { }, ); + // ======================================================================== + // Spaces in quoted vault names and secret keys (#902) + // ======================================================================== + + await t.step( + "should resolve vault expression with spaces in double-quoted key", + async () => { + const resolver = createResolverWithMockVault({ + "Client ID": "my-client-id", + }); + const result = await resolver.resolveVaultExpressions( + 'vault.get("test-vault", "Client ID")', + ); + assertEquals(result, '"my-client-id"'); + }, + ); + + await t.step( + "should resolve vault expression with spaces in single-quoted key", + async () => { + const resolver = createResolverWithMockVault({ + "Client Secret": "super-secret", + }); + const result = await resolver.resolveVaultExpressions( + "vault.get('test-vault', 'Client Secret')", + ); + assertEquals(result, '"super-secret"'); + }, + ); + + await t.step( + "should resolve vault expression with 1Password-style path containing spaces", + async () => { + const resolver = createResolverWithMockVault({ + "Tailscale K8s Operator/Client ID": "ts-client-id-123", + }); + const result = await resolver.resolveVaultExpressions( + 'vault.get("test-vault", "Tailscale K8s Operator/Client ID")', + ); + assertEquals(result, '"ts-client-id-123"'); + }, + ); + + await t.step( + "should resolve vault expression with spaces in quoted vault name", + async () => { + const resolver = createResolverWithNamedVaults({ + "my infra vault": { "api-key": "infra-key-456" }, + }); + const result = await resolver.resolveVaultExpressions( + 'vault.get("my infra vault", "api-key")', + ); + assertEquals(result, '"infra-key-456"'); + }, + ); + + await t.step( + "should resolve vault expression with spaces in both vault name and key", + async () => { + const resolver = createResolverWithNamedVaults({ + "my infra vault": { "Client Secret": "both-spaces-val" }, + }); + const result = await resolver.resolveVaultExpressions( + 'vault.get("my infra vault", "Client Secret")', + ); + assertEquals(result, '"both-spaces-val"'); + }, + ); + + await t.step( + "should resolve mixed quoted vault name with unquoted key", + async () => { + const resolver = createResolverWithNamedVaults({ + "my infra vault": { "api-key": "mixed-val" }, + }); + const result = await resolver.resolveVaultExpressions( + 'vault.get("my infra vault", api-key)', + ); + assertEquals(result, '"mixed-val"'); + }, + ); + // Cleanup await Deno.remove(tempDir, { recursive: true }); });