fix(core): Accept mixed dot and bracket notation in external secret expressions#28563
Open
jeanibarz wants to merge 1 commit inton8n-io:masterfrom
Open
fix(core): Accept mixed dot and bracket notation in external secret expressions#28563jeanibarz wants to merge 1 commit inton8n-io:masterfrom
jeanibarz wants to merge 1 commit inton8n-io:masterfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
There was a problem hiding this comment.
No issues found across 2 files
Architecture diagram
sequenceDiagram
participant UI as Client / API Caller
participant CS as Credentials Service
participant VAL as Validator (validation.ts)
participant UT as Utils (external-secrets.utils.ts)
Note over UI,UT: Request to save credentials with expression:<br/>$secrets.azureKeyVault["postgres-n8n-data"]
UI->>CS: Save Credentials Request
CS->>VAL: validateAccessToReferencedSecretProviders(expression)
VAL->>UT: extractProviderKeysFromExpression(expression)
Note over UT: CHANGED: Regex now looks ahead for<br/>either "." or "[" after provider name
UT->>UT: Match $secrets.(${PROVIDER_KEY_PATTERN})(?=[.\[])
alt Successful Match (Fixed Path)
UT-->>VAL: Return ["azureKeyVault"]
VAL->VAL: Verify user has access to "azureKeyVault"
VAL-->>CS: Validation Success
CS-->>UI: 200 OK (Credentials Saved)
else No Match (Regression/Error Path)
UT-->>VAL: Return []
VAL-->>CS: Throw "Could not find a valid external secret vault name"
CS-->>UI: 400 Bad Request / Error
end
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Credentials using the external secrets vault can't be saved when an expression combines dot and bracket notation, e.g.
$secrets.azureKeyVault["postgres-n8n-data"]. The save request fails withCould not find a valid external secret vault name, and the only workaround is to rewrite every affected expression as pure bracket notation.The root cause is in
extractProviderKeysFromExpressioninpackages/cli/src/credentials/external-secrets.utils.ts. The dot-notation regex uses a(?=\.)lookahead, so it requires the character after the provider key to be another dot. A mixed expression like$secrets.azureKeyVault["postgres-n8n-data"]has[there, so neither the dot regex nor the bracket regex (which expects$secrets['vault']) matches, andextractProviderKeysFromExpressionreturns[].validateAccessToReferencedSecretProvidersincredentials/validation.tsthen treats the expression as having no valid provider and throws.This regression was introduced by #25406 (release 2.9.0) when the validator was added.
The fix
Widen the dot-notation lookahead from
(?=\.)to(?=[.\[]), so it accepts either further dot access ($secrets.vault.key) or a transition into bracket notation ($secrets.vault["key"]). The new regex is a strict superset of the old one, so existing working expressions are unaffected.Key decisions
$secrets["vault"]["key"]) already works via the separate bracket regex, so no change there. The analogous mirror-image case$secrets["vault"].keyis a separate shape that would need its own fix — I'm intentionally not scope-creeping into it here.external-secrets.utils.tsneeds to change;containsExternalSecretExpressionandvalidation.tsdon't require updates because they already detect the$secrets.prefix and dispatch to the extractor.Tests
Added five unit tests in
external-secrets.utils.test.tscovering the new surface:$secrets.my-vault["key"])$secrets.vault["key"].subfield)All five fail on master and pass with the fix. The existing 12 tests in
external-secrets.utils.test.tsand 39 tests incredentials/validation.test.tsremain green.Related Linear tickets, Github issues, and Community forum posts
closes #28516
Review / Merge checklist