security: close all 40 open CodeQL alerts (v2.47.3)#709
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
Test Results Summary📊 ArtifactsGenerated at Wed, 08 Apr 2026 08:15:59 GMT |
czlonkowski
added a commit
that referenced
this pull request
Apr 8, 2026
PR #709's first CodeQL run surfaced two alerts that my earlier fixes didn't actually close: 1. **cache-utils.ts:218 — `js/insufficient-password-hash`** I had switched `createCacheKey` from plain SHA256 to HMAC-SHA256, but CodeQL's rule accepts only actual KDFs (bcrypt, scrypt, argon2, pbkdf2 with ≥10k iterations) — HMAC is not on the allowlist. Switched to `scryptSync` with intentionally low cost parameters (`N=1024, r=8, p=1`, ~2–5 ms per call). This is on CodeQL's allowlist and the per-call cost is bounded by the existing memoization — each unique input runs scrypt exactly once per process lifetime, so cache hits are still O(1). The threat model is "don't leak credentials via cache keys in logs or memory dumps", not "resist offline brute force", so we don't need bcrypt-level cost. The per-process random salt also means cache keys don't survive a server restart. 2. **test-docker-fingerprint.ts:148 — `js/clear-text-logging`** The previous fix masked `N8N_MCP_USER_ID` by logging only its first/last chars via `slice()`. CodeQL's taint tracking follows string slicing — any substring of a sensitive source is still flagged as sensitive. Replaced with a constant string ("is set (value redacted)") that contains no data derived from the env var. The only signal a developer needs from this line is "an override is set", which the new text conveys. Verified: - 4512 unit tests pass (scrypt test suite adds ~1.8s of overhead) - npm run typecheck clean - npm run build clean Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Hardening pass covering the CodeQL rules listed in the CHANGELOG.
No runtime behaviour change beyond what the individual fix comments
at each site document. 4512 unit tests and 699 integration tests pass.
Notable by-products:
- Shared linear-time expression-extraction helpers in
src/utils/expression-utils.ts for validators that previously used
lazy-quantifier regexes.
- Chat trigger session ID format is now session_{ts}_{UUIDv4};
the matching test regex was updated.
- scripts/update-n8n-deps.js and related scripts are unchanged;
all fixes are under src/, tests/, and scripts/.
Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
czlonkowski
force-pushed
the
security/codeql-fixes
branch
from
f902ed7 to
9f89d56
Compare
RenatoAscencio
pushed a commit
to serversmx/n8n-mcp
that referenced
this pull request
May 6, 2026
Hardening pass covering the CodeQL rules listed in the CHANGELOG.
No runtime behaviour change beyond what the individual fix comments
at each site document. 4512 unit tests and 699 integration tests pass.
Notable by-products:
- Shared linear-time expression-extraction helpers in
src/utils/expression-utils.ts for validators that previously used
lazy-quantifier regexes.
- Chat trigger session ID format is now session_{ts}_{UUIDv4};
the matching test regex was updated.
- scripts/update-n8n-deps.js and related scripts are unchanged;
all fixes are under src/, tests/, and scripts/.
Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
2 participants
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
Hardening pass that closes the open CodeQL alerts on
main. None of the findings were introduced by recent work — they were surfaced when CodeQL first ran fresh after the earlier refactor landed.Rules addressed:
js/regex-injection,js/prototype-polluting-assignment,js/prototype-pollution-utility,js/double-escaping,js/polynomial-redos,js/insufficient-password-hash,js/insecure-randomness,js/clear-text-logging,js/tainted-format-string,js/incomplete-url-substring-sanitization,js/shell-command-injection-from-environment.Reviewer note: I am deliberately not describing the mechanics of each finding in this description. Per-site rationale lives in inline comments at the fix locations for maintainers who need it; the CodeQL Security tab has the full alert details for anyone with repo access. For a public OSS repo, CHANGELOG + commit message + PR description should not read as an exploitation roadmap for versions that haven't been upgraded yet.
Notable by-products (no vuln mechanics)
extractBracketExpressions()/hasBracketExpression()helpers insrc/utils/expression-utils.ts. Replaces several lazy-quantifier regexes across the expression and workflow validators. Reusable by future validators.createCacheKeyinsrc/utils/cache-utils.tsmoved to a CodeQL-approved KDF with aggressive memoization — O(1) on cache hits, one KDF call per unique input per process. Semantics preserved.session_{timestamp}_{9-char-alnum}tosession_{timestamp}_{UUIDv4}. Matching test regex updated.createCacheKey's internals locally now call the real function — tracks production behaviour and avoids drift.Test plan
npm run typecheckcleannpm run buildcleansecretlint "**/*" "**/.*"— exit 0data/nodes.dbnot touchedWhy this is one bundled PR and not eleven
The user explicitly said they prefer bundled refactors over churn when the changes are in related code, and every fix here is a surgical hardening of existing code paths with no user-facing behaviour change. Separating into 11 PRs would multiply review overhead without improving bisectability.
Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en