fix: split checksums on any line ending so LF files parse on Windows#443
Merged
Merged
Conversation
getSubjectFromChecksumsString split the checksums input on os.EOL. On Windows os.EOL is "\r\n", so an LF-only checksums file never split: the whole file collapsed into a single record, only the first artifact was attested, and every other listed file was silently left unattested while the step still succeeded. Split on /\r?\n/ instead so the parser is line-ending agnostic, and add a defense-in-depth guard that rejects any subject name containing a newline so a malformed multi-line record fails loudly instead of producing a partial attestation. Regenerated dist/ to match. Fixes #440 Co-authored-by: Copilot App <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes cross-platform parsing of subject-checksums input so checksum files with LF line endings are correctly parsed on Windows, preventing silent partial attestations.
Changes:
- Parse checksums records using a line-ending agnostic split (
/\r?\n/) and remove the unusedosimport. - Add a defense-in-depth validation to reject subject names containing newline characters.
- Add regression tests for LF-only input on Windows, CRLF input, and newline-containing subject names; update bundled
dist/.
Show a summary per file
| File | Description |
|---|---|
| src/subject.ts | Makes checksums parsing line-ending agnostic and adds a guard against multiline subject names. |
| dist/index.js | Updates the bundled output to reflect the src/subject.ts changes. |
| tests/unit/subject.test.ts | Adds regression tests covering LF, CRLF, and newline-in-name failure behavior. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/3 changed files
- Comments generated: 1
- Review effort level: Low
Comment on lines
+209
to
+211
| if (/[\r\n]/.test(name)) { | ||
| throw new Error(`Invalid subject name (contains a newline): ${name}`) | ||
| } |
tingx2wang
approved these changes
Jul 8, 2026
This was referenced 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
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
Fixes a bug where an LF-only checksums file is not parsed correctly on Windows, causing all but the first listed artifact to be silently left unattested while the action still succeeds.
Bug
getSubjectFromChecksumsStringinsrc/subject.tssplit the checksums input using the platform-specific line ending:Root cause
On Windows
os.EOLis\r\n. An LF-only (\n) checksums file therefore never splits — the entire file becomes a single record. The first 64 hex characters are taken as the digest and everything after the first space (including all newlines and subsequent lines) becomes the subjectname. Result: only the first artifact is attested and every other listed file is silently dropped, with the step still reporting success.Fix
/\r?\n/so parsing is line-ending agnostic (handles LF, CRLF, and mixed inputs on any platform).osimport fromsrc/subject.ts.Defense in depth
After computing each subject
name, reject any name containing a newline (\nor\r) by throwing a descriptive error. If input ever fails to split correctly again, this turns a silent partial attestation into a loud failure.Tests
Added regression tests in
__tests__/unit/subject.test.ts:npm run lint,npm test(112 passing, 100% coverage), andnpm run bundleall pass; the regenerateddist/is committed so thecheck-distCI check stays green.Workaround
Until this is released, affected users can switch from
subject-checksumstosubject-path.Fixes #440