Skip to content

Fix backslashes being accepted by sanitizeArtifactName #2922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 4, 2025

Conversation

mbg
Copy link
Member

@mbg mbg commented Jun 4, 2025

Artifact names may not contain backslashes, but sanitizeArtifactName accepts them, potentially resulting in errors during debug artifact uploads:

Warning: Failed to upload debug artifacts: Error: The artifact name is not valid: foo\\bar Contains the following character:  Backslash \

This PR removes them from the list of acceptable characters in the regex and adds a test case.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@Copilot Copilot AI review requested due to automatic review settings June 4, 2025 11:29
@mbg mbg requested a review from a team as a code owner June 4, 2025 11:29
Copy link
Contributor

@Copilot 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

This PR tightens the sanitizeArtifactName regex to disallow backslashes and adds a corresponding test case to prevent debug upload errors.

  • Removed backslash (\) from allowed characters in the regex for artifact names.
  • Added a test case to verify backslashes are stripped out.
  • Updated both TypeScript sources and compiled JavaScript outputs to keep them in sync.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

File Description
src/debug-artifacts.ts Updated regex to disallow backslashes in names.
src/debug-artifacts.test.ts Added test case for backslashes in names.
lib/debug-artifacts.js Mirrored regex update in compiled output.
lib/debug-artifacts.test.js Mirrored test addition in compiled tests.
Comments suppressed due to low confidence (1)

src/debug-artifacts.test.ts:21

  • Consider adding a test case to ensure valid characters like hyphens and underscores are preserved (e.g., foo-bar_bazfoo-bar_baz).
t.deepEqual(

@mbg mbg merged commit 72be4b6 into main Jun 4, 2025
272 checks passed
@mbg mbg deleted the mbg/fix/sanitizeArtifactName branch June 4, 2025 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants