-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Check for newlines and carriage return in artifact paths and name #951
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1c2a1f1
Check for newlines and carriage return in artifact paths and name
konradpabjan 68929d2
Fix linting issue
konradpabjan c638ec0
Update comments
konradpabjan df0a8cc
Add comment about spacing
konradpabjan 3d5c206
Remove extra space
konradpabjan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
78 changes: 78 additions & 0 deletions
78
packages/artifact/__tests__/path-and-artifact-name-validation.test.ts
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
import { | ||
checkArtifactName, | ||
checkArtifactFilePath | ||
} from '../src/internal/path-and-artifact-name-validation' | ||
import * as core from '@actions/core' | ||
|
||
describe('Path and artifact name validation', () => { | ||
beforeAll(() => { | ||
// mock all output so that there is less noise when running tests | ||
jest.spyOn(console, 'log').mockImplementation(() => {}) | ||
jest.spyOn(core, 'debug').mockImplementation(() => {}) | ||
jest.spyOn(core, 'info').mockImplementation(() => {}) | ||
jest.spyOn(core, 'warning').mockImplementation(() => {}) | ||
}) | ||
|
||
it('Check Artifact Name for any invalid characters', () => { | ||
const invalidNames = [ | ||
'my\\artifact', | ||
'my/artifact', | ||
'my"artifact', | ||
'my:artifact', | ||
'my<artifact', | ||
'my>artifact', | ||
'my|artifact', | ||
'my*artifact', | ||
'my?artifact', | ||
'' | ||
] | ||
for (const invalidName of invalidNames) { | ||
expect(() => { | ||
checkArtifactName(invalidName) | ||
}).toThrow() | ||
} | ||
|
||
const validNames = [ | ||
'my-normal-artifact', | ||
'myNormalArtifact', | ||
'm¥ñðrmålÄr†ï£å¢†' | ||
] | ||
for (const validName of validNames) { | ||
expect(() => { | ||
checkArtifactName(validName) | ||
}).not.toThrow() | ||
} | ||
}) | ||
|
||
it('Check Artifact File Path for any invalid characters', () => { | ||
const invalidNames = [ | ||
'some/invalid"artifact/path', | ||
'some/invalid:artifact/path', | ||
'some/invalid<artifact/path', | ||
'some/invalid>artifact/path', | ||
'some/invalid|artifact/path', | ||
'some/invalid*artifact/path', | ||
'some/invalid?artifact/path', | ||
'some/invalid\rartifact/path', | ||
'some/invalid\nartifact/path', | ||
'some/invalid\r\nartifact/path', | ||
'' | ||
] | ||
for (const invalidName of invalidNames) { | ||
expect(() => { | ||
checkArtifactFilePath(invalidName) | ||
}).toThrow() | ||
} | ||
|
||
const validNames = [ | ||
'my/perfectly-normal/artifact-path', | ||
'my/perfectly\\Normal/Artifact-path', | ||
'm¥/ñðrmål/Är†ï£å¢†' | ||
] | ||
for (const validName of validNames) { | ||
expect(() => { | ||
checkArtifactFilePath(validName) | ||
}).not.toThrow() | ||
} | ||
}) | ||
}) |
This file contains 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
This file contains 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
82 changes: 82 additions & 0 deletions
82
packages/artifact/src/internal/path-and-artifact-name-validation.ts
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
import {info} from '@actions/core' | ||
|
||
/** | ||
* Invalid characters that cannot be in the artifact name or an uploaded file. Will be rejected | ||
* from the server if attempted to be sent over. These characters are not allowed due to limitations with certain | ||
* file systems such as NTFS. To maintain platform-agnostic behavior, all characters that are not supported by an | ||
* individual filesystem/platform will not be supported on all fileSystems/platforms | ||
* | ||
* FilePaths can include characters such as \ and / which are not permitted in the artifact name alone | ||
*/ | ||
const invalidArtifactFilePathCharacters = new Map<string, string>([ | ||
['"', ' Double quote "'], | ||
[':', ' Colon :'], | ||
['<', ' Less than <'], | ||
['>', ' Greater than >'], | ||
['|', ' Vertical bar |'], | ||
['*', ' Asterisk *'], | ||
['?', ' Question mark ?'], | ||
['\r', ' Carriage return \\r'], | ||
['\n', ' Line feed \\n'] | ||
]) | ||
|
||
const invalidArtifactNameCharacters = new Map<string, string>([ | ||
...invalidArtifactFilePathCharacters, | ||
['\\', ' Backslash \\'], | ||
robherley marked this conversation as resolved.
Show resolved
Hide resolved
|
||
['/', ' Forward slash /'] | ||
]) | ||
|
||
/** | ||
* Scans the name of the artifact to make sure there are no illegal characters | ||
*/ | ||
export function checkArtifactName(name: string): void { | ||
if (!name) { | ||
throw new Error(`Artifact name: ${name}, is incorrectly provided`) | ||
} | ||
|
||
for (const [ | ||
invalidCharacterKey, | ||
errorMessageForCharacter | ||
] of invalidArtifactNameCharacters) { | ||
if (name.includes(invalidCharacterKey)) { | ||
throw new Error( | ||
`Artifact name is not valid: ${name}. Contains the following character: ${errorMessageForCharacter} | ||
|
||
Invalid characters include: ${Array.from( | ||
invalidArtifactNameCharacters.values() | ||
).toString()} | ||
|
||
These characters are not allowed in the artifact name due to limitations with certain file systems such as NTFS. To maintain file system agnostic behavior, these characters are intentionally not allowed to prevent potential problems with downloads on different file systems.` | ||
) | ||
} | ||
} | ||
|
||
info(`Artifact name is valid!`) | ||
} | ||
|
||
/** | ||
* Scans the name of the filePath used to make sure there are no illegal characters | ||
*/ | ||
export function checkArtifactFilePath(path: string): void { | ||
if (!path) { | ||
throw new Error(`Artifact path: ${path}, is incorrectly provided`) | ||
} | ||
|
||
for (const [ | ||
invalidCharacterKey, | ||
errorMessageForCharacter | ||
] of invalidArtifactFilePathCharacters) { | ||
if (path.includes(invalidCharacterKey)) { | ||
throw new Error( | ||
`Artifact path is not valid: ${path}. Contains the following character: ${errorMessageForCharacter} | ||
|
||
Invalid characters include: ${Array.from( | ||
invalidArtifactFilePathCharacters.values() | ||
).toString()} | ||
|
||
The following characters are not allowed in files that are uploaded due to limitations with certain file systems such as NTFS. To maintain file system agnostic behavior, these characters are intentionally not allowed to prevent potential problems with downloads on different file systems. | ||
` | ||
) | ||
} | ||
} | ||
} |
This file contains 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
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New tests are here, I just moved everything into a separate file for better organization