Skip to content

Harden ATA package name filtering#63368

Merged
jakebailey merged 2 commits intomicrosoft:mainfrom
jakebailey:fix-package-sanitization
Apr 7, 2026
Merged

Harden ATA package name filtering#63368
jakebailey merged 2 commits intomicrosoft:mainfrom
jakebailey:fix-package-sanitization

Conversation

@jakebailey
Copy link
Copy Markdown
Member

This URI-based validation is what npm does internally, but we should be stricter about what we allow down to npm as we use child_process with the shell enabled to avoid reimplementing which. Instead, just only allow [\w.-]+.

This prevents the odd scenario where someone manages to query ATA on via a broken specifier. The old URI-based validation allowing something through would maximally cause the ATA request to fail and the feature to not work, nothing further.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Tightens the typings installer’s package-name validation to a strict allowlist before constructing npm install command strings (shell-enabled), and updates diagnostics + baselines accordingly.

Changes:

  • Replace URI-encoding-based package name validation with an explicit ^[\w.-]+$ allowlist (plus existing leading ./_ checks and scoped-name splitting).
  • Introduce NameContainsInvalidCharacters (with an enum alias to preserve the old NameContainsNonURISafeCharacters name) and update the rendered failure message.
  • Update unit tests and tsserver baseline logs to match the new validation result/message.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/jsTyping/jsTyping.ts Implements the new allowlist validation, adds/aliases enum member, updates rendered error text.
src/testRunner/unittests/tsserver/typingsInstaller.ts Updates/extends validation tests to expect the new enum result.
tests/baselines/reference/tsserver/typingsInstaller/should-not-initialize-invaalid-package-names.js Baseline update for new error message text.
tests/baselines/reference/tsserver/typingsInstaller/should-handle-node-core-modules.js Baseline update for new error message text.
tests/baselines/reference/tsserver/typingsInstaller/malformed-packagejson.js Baseline update for new error message text.

@github-project-automation github-project-automation bot moved this from Not started to Needs merge in PR Backlog Apr 7, 2026
@jakebailey jakebailey added this pull request to the merge queue Apr 7, 2026
Merged via the queue into microsoft:main with commit c7a0ae1 Apr 7, 2026
23 checks passed
@jakebailey jakebailey deleted the fix-package-sanitization branch April 7, 2026 17:01
@github-project-automation github-project-automation bot moved this from Needs merge to Done in PR Backlog Apr 7, 2026
@jakebailey
Copy link
Copy Markdown
Member Author

@typescript-bot cherry-pick this to release-6.0

@typescript-bot
Copy link
Copy Markdown
Collaborator

typescript-bot commented Apr 7, 2026

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
cherry-pick this to release-6.0 ✅ Started ✅ Results

@typescript-bot
Copy link
Copy Markdown
Collaborator

Hey, @jakebailey! I've created #63372 for you.

jakebailey added a commit that referenced this pull request Apr 7, 2026
…#63372)

Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants