fix: resolve CodeQL code-scanning security alerts#140
Merged
laileni-aws merged 4 commits intoJul 15, 2026
Conversation
Problem:
17 open CodeQL code-scanning alerts across source, tests, dev scripts,
and a workflow (ReDoS, incomplete sanitization, shell-command injection
from environment, and a missing workflow permissions block).
Solution:
- ReDoS (js/polynomial-redos): rewrite vulnerable regexes as linear
equivalents - indent()/block-comment/base64url padding in
textUtilities.ts; use-strict/require detection in importAdderUtil.ts;
trailing '/' and '#' stripping in auth.ts and authUtil.ts (behavior
verified against existing unit tests).
- Shell injection (js/shell-command-injection-from-environment): replace
execSync(string) with execFileSync('git', [...args]) (no shell) in
scripts/createRelease.ts and scripts/newChange.ts.
- Incomplete sanitization (js/incomplete-sanitization): replace all
quotes, not just the first, in ssh.test.ts.
- URL substring checks (js/incomplete-url-substring-sanitization): use
assert.match instead of includes() on URLs in
referenceLogViewProvider.test.ts.
- Workflow permissions (actions/missing-workflow-permissions): add a
least-privilege top-level 'contents: read' to release.yml (the publish
job keeps its explicit 'contents: write').
- Certificate validation (js/disabling-certificate-validation): this is
an explicit user opt-in mirroring VS Code's 'http.proxyStrictSSL';
kept the behavior, upgraded the log to a warning, and documented it.
None of these overlap with the dependency PRs (aws#136-138).
The anchored /^\s*use\s+strict/ regex missed quoted directives like 'use strict'; (line starts with a quote), breaking importAdderUtil 'first line after comments and use strict'. Allow an optional leading quote: /^\s*['"]?use\s+strict/ (still anchored/linear).
laileni-aws
marked this pull request as ready for review
July 14, 2026 21:56
kmmcclai
reviewed
Jul 14, 2026
Add the global flag to the ReDoS-hardened block-comment regex so lines with multiple block comments are fully stripped (and code between them is preserved), matching the function's documented intent. Addresses review feedback on aws#140; the g flag keeps the regex linear (no ReDoS).
kmmcclai
approved these changes
Jul 14, 2026
laileni-aws
enabled auto-merge (squash)
July 15, 2026 00:02
XiaoChen-amz
approved these changes
Jul 15, 2026
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
3 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.
Problem
There are 17 open CodeQL code-scanning alerts (separate from Dependabot):
ReDoS-prone regexes, incomplete sanitization, shell-command injection from
environment in dev scripts, an incomplete URL-substring check in a test, and a
workflow missing an explicit
permissionsblock. None overlap with thedependency PRs (#136 / #137 / #138), which only changed lockfiles.
Solution
Fix each alert at the source with minimal, behavior-preserving changes. Regex
rewrites were verified against the existing unit tests (
indent(),reformatStartUrl) plus ad-hoc equivalence checks.js/polynomial-redostextUtilities.tsindent()(\n+)→(\n); block-comment regex → linear\/\*(?:[^*]|\*(?!\/))*\*\/; base64url/=+$/→ linear loopjs/polynomial-redosimportAdderUtil.ts^); simplifyrequire(detectionjs/polynomial-redosauth.ts//#strip → linear loopjs/polynomial-redosauthUtil.ts//#strip → linear loopjs/shell-command-injection-from-environmentscripts/createRelease.tsexecSync(string)→execFileSync('git', [...])(no shell)js/shell-command-injection-from-environmentscripts/newChange.tsexecSync(string)→execFileSync('git', [...])js/incomplete-sanitizationssh.test.ts.replace('"','')→.replace(/"/g,'')js/incomplete-url-substring-sanitizationreferenceLogViewProvider.test.tsassert.ok(x.includes(url))→assert.match(x, /.../)actions/missing-workflow-permissions.github/workflows/release.ymlpermissions: contents: readjs/disabling-certificate-validationproxyUtil.tsNote on #10 (
proxyUtil.ts)Disabling TLS verification here is an explicit, user opt-in that mirrors VS
Code's own
http.proxyStrictSSLsetting (used for corporate proxies withself-signed certs). Removing it would drop a legitimate user feature, so the
behavior is kept — the log was upgraded to a warning and the intent
documented. If CodeQL still flags it, recommend dismissing as
"won't fix (by design)".
Verification
(
indent()5 cases,reformatStartUrl6 cases) and base64url equivalence —all pass.
feature/xbranches will not be squash-merged at release time.