Merge e94b1128e6 into 12cd2235ef

* trim only ascii whitespace for branch

* rebuild

README.md

@@ -36,6 +36,7 @@ When Git 2.18 or higher is not in your PATH, falls back to the REST API to downl
 
 ### Note
 
+
 Thank you for your interest in this GitHub action, however, right now we are not taking contributions. 
 
 We continue to focus our resources on strategic areas that help our customers be successful while making developers' lives easier. While GitHub Actions remains a key part of this vision, we are allocating resources towards other areas of Actions and are not taking contributions to this repository at this time. The GitHub public roadmap is the best place to follow along for any updates on features we’re working on and what stage they’re in.

__test__/input-helper.test.ts

@@ -24,9 +24,20 @@ const mockGithubContext: any = {
   payload: {}
 }
 
+// Replicate @actions/core getInput behavior: it trims whitespace by default
+// (String.prototype.trim(), which strips characters such as a leading U+FEFF BOM)
+// unless trimWhitespace is explicitly set to false.
+const getInputImpl = (name: string, options?: {trimWhitespace?: boolean}) => {
+  const val = inputs[name] ?? ''
+  if (options && options.trimWhitespace === false) {
+    return val
+  }
+  return typeof val === 'string' ? val.trim() : val
+}
+
 // Mock @actions/core before loading input-helper
 jest.unstable_mockModule('@actions/core', () => ({
-  getInput: jest.fn((name: string) => inputs[name]),
+  getInput: jest.fn(getInputImpl),
   getBooleanInput: jest.fn((name: string) => inputs[name]),
   getMultilineInput: jest.fn((name: string) =>
     inputs[name] ? String(inputs[name]).split('\n').filter(Boolean) : []
@@ -76,9 +87,7 @@ describe('input-helper tests', () => {
     inputs = {}
     jest.clearAllMocks()
     // Re-apply default mocks
-    ;(core.getInput as jest.Mock<any>).mockImplementation(
-      (name: string) => inputs[name]
-    )
+    ;(core.getInput as jest.Mock<any>).mockImplementation(getInputImpl as any)
     mockDirectoryExistsSync.mockImplementation(
       (p: string) => p === gitHubWorkspace
     )
@@ -176,8 +185,84 @@ describe('input-helper tests', () => {
     expect(settings.commit).toBeFalsy()
   })
 
+  it('does not reclassify a ref as sha when a BOM is prefixed', async () => {
+    // A fork branch named "<U+FEFF>" + 40 hex chars. core.getInput trims the
+    // BOM by default, which previously collapsed this into a bare SHA and
+    // bypassed the unsafe fork PR checkout guard.
+    inputs.ref = '\uFEFF522d932fae5296da51fdf431934425ecf891c6a2'
+    const settings: IGitSourceSettings = await inputHelper.getInputs()
+    expect(settings.commit).toBeFalsy()
+    expect(settings.ref).toBe('522d932fae5296da51fdf431934425ecf891c6a2')
+  })
+
+  it('does not reclassify a sha-256 ref as sha when a BOM is prefixed', async () => {
+    inputs.ref =
+      '\uFEFF1111111111222222222233333333334444444444555555555566666666667777'
+    const settings: IGitSourceSettings = await inputHelper.getInputs()
+    expect(settings.commit).toBeFalsy()
+    expect(settings.ref).toBe(
+      '1111111111222222222233333333334444444444555555555566666666667777'
+    )
+  })
+
+  it('treats a sha surrounded by ascii whitespace as a commit', async () => {
+    // ASCII whitespace can only come from the workflow author's YAML (git ref
+    // names cannot contain it), so trimming it and treating the value as a
+    // commit is safe.
+    inputs.ref = '  1111111111222222222233333333334444444444  '
+    const settings: IGitSourceSettings = await inputHelper.getInputs()
+    expect(settings.ref).toBeFalsy()
+    expect(settings.commit).toBe('1111111111222222222233333333334444444444')
+  })
+
   it('sets workflow organization ID', async () => {
     const settings: IGitSourceSettings = await inputHelper.getInputs()
     expect(settings.workflowOrganizationId).toBe(123456)
   })
+
+  describe('unsafe PR checkout guard', () => {
+    const forkPayload = {
+      repository: {id: 100},
+      pull_request: {
+        head: {
+          sha: '1234567890123456789012345678901234567890',
+          repo: {id: 200, full_name: 'attacker/fork'}
+        },
+        merge_commit_sha: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
+      }
+    }
+
+    it('allows the default self-checkout on a fork pull_request_target', async () => {
+      const originalEvent = mockGithubContext.eventName
+      const originalPayload = mockGithubContext.payload
+      try {
+        mockGithubContext.eventName = 'pull_request_target'
+        mockGithubContext.payload = forkPayload
+        // Simulate a rebase/fast-forward merge where the base tip (event SHA)
+        // equals the PR head SHA. The default self-checkout must still succeed.
+        mockGithubContext.sha = '1234567890123456789012345678901234567890'
+        const settings: IGitSourceSettings = await inputHelper.getInputs()
+        expect(settings.commit).toBe('1234567890123456789012345678901234567890')
+      } finally {
+        mockGithubContext.eventName = originalEvent
+        mockGithubContext.payload = originalPayload
+      }
+    })
+
+    it('refuses an explicit fork repository on pull_request_target', async () => {
+      const originalEvent = mockGithubContext.eventName
+      const originalPayload = mockGithubContext.payload
+      try {
+        mockGithubContext.eventName = 'pull_request_target'
+        mockGithubContext.payload = forkPayload
+        inputs.repository = 'attacker/fork'
+        await expect(inputHelper.getInputs()).rejects.toThrow(
+          /Refusing to check out fork pull request code/
+        )
+      } finally {
+        mockGithubContext.eventName = originalEvent
+        mockGithubContext.payload = originalPayload
+      }
+    })
+  })
 })

dist/index.js

@@ -42100,6 +42100,22 @@ async function getInputs() {
         `${github_context.repo.owner}/${github_context.repo.repo}`.toUpperCase();
     // Source branch, source version
     result.ref = getInput('ref');
+    // core.getInput()'s default trim strips a range of Unicode characters such as a
+    // leading BOM (U+FEFF) or NBSP (U+00A0). Those are valid in a git ref name, so
+    // a fork branch named "<BOM>" + 40 hex chars would trim down to a bare SHA and
+    // be silently reclassified as a commit, bypassing the unsafe fork PR checkout
+    // guard.
+    //
+    // The trim below strips only the ASCII whitespace characters which are all forbidden
+    // in a git branch name.
+    //   \t  U+0009  horizontal tab   - ASCII control, forbidden in ref names
+    //   \n  U+000A  line feed        - ASCII control, forbidden in ref names
+    //   \v  U+000B  vertical tab     - ASCII control, forbidden in ref names
+    //   \f  U+000C  form feed        - ASCII control, forbidden in ref names
+    //   \r  U+000D  carriage return  - ASCII control, forbidden in ref names
+    //   ' ' U+0020  space            - forbidden in ref names
+    const asciiTrimmedRef = getInput('ref', { trimWhitespace: false })
+        .replace(/^[\t\n\v\f\r ]+|[\t\n\v\f\r ]+$/g, '');
     if (!result.ref) {
         if (isWorkflowRepository) {
             result.ref = github_context.ref;
@@ -42112,8 +42128,8 @@ async function getInputs() {
         }
     }
     // SHA?
-    else if (result.ref.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) {
-        result.commit = result.ref;
+    else if (asciiTrimmedRef.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) {
+        result.commit = asciiTrimmedRef;
         result.ref = '';
     }
     core_debug(`ref = '${result.ref}'`);
@@ -42191,12 +42207,19 @@ async function getInputs() {
         (getInput('allow-unsafe-pr-checkout') || 'false').toUpperCase() ===
             'TRUE';
     core_debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`);
-    assertSafePrCheckout({
-        qualifiedRepository,
-        ref: result.ref,
-        commit: result.commit,
-        allowUnsafePrCheckout: result.allowUnsafePrCheckout
-    });
+    // The default self-checkout (this repository with no explicit ref) always
+    // resolves to the trusted ref/commit GitHub set for the triggering event, so
+    // the fork-checkout guard only needs to run when the caller customized the
+    // repository or ref.
+    const isDefaultCheckout = isWorkflowRepository && !getInput('ref');
+    if (!isDefaultCheckout) {
+        assertSafePrCheckout({
+            qualifiedRepository,
+            ref: result.ref,
+            commit: result.commit,
+            allowUnsafePrCheckout: result.allowUnsafePrCheckout
+        });
+    }
     return result;
 }
 

pr.yaml

@@ -0,0 +1,23 @@
+---
+# This workflow will execute on pull request to dev branch
+
+name: pr-workflow
+
+on:
+    pull_request:
+        branches:
+          - dev
+
+jobs:
+    build:
+        runs-on: ubuntu-latest
+        steps:
+            - name: get the code
+              uses: actions/checkout@v6
+            - name: compile the code
+              run: mvn compile
+            - name: test the code
+              run: mvn test
+            - name: create the package
+              run: mvn package
+              

src/input-helper.ts

@@ -59,6 +59,23 @@ export async function getInputs(): Promise<IGitSourceSettings> {
 
   // Source branch, source version
   result.ref = core.getInput('ref')
+  // core.getInput()'s default trim strips a range of Unicode characters such as a
+  // leading BOM (U+FEFF) or NBSP (U+00A0). Those are valid in a git ref name, so
+  // a fork branch named "<BOM>" + 40 hex chars would trim down to a bare SHA and
+  // be silently reclassified as a commit, bypassing the unsafe fork PR checkout
+  // guard.
+  //
+  // The trim below strips only the ASCII whitespace characters which are all forbidden
+  // in a git branch name.
+  //   \t  U+0009  horizontal tab   - ASCII control, forbidden in ref names
+  //   \n  U+000A  line feed        - ASCII control, forbidden in ref names
+  //   \v  U+000B  vertical tab     - ASCII control, forbidden in ref names
+  //   \f  U+000C  form feed        - ASCII control, forbidden in ref names
+  //   \r  U+000D  carriage return  - ASCII control, forbidden in ref names
+  //   ' ' U+0020  space            - forbidden in ref names
+  const asciiTrimmedRef = core
+    .getInput('ref', {trimWhitespace: false})
+    .replace(/^[\t\n\v\f\r ]+|[\t\n\v\f\r ]+$/g, '')
   if (!result.ref) {
     if (isWorkflowRepository) {
       result.ref = github.context.ref
@@ -72,8 +89,8 @@ export async function getInputs(): Promise<IGitSourceSettings> {
     }
   }
   // SHA?
-  else if (result.ref.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) {
-    result.commit = result.ref
+  else if (asciiTrimmedRef.match(/^(?:[0-9a-fA-F]{40}|[0-9a-fA-F]{64})$/)) {
+    result.commit = asciiTrimmedRef
     result.ref = ''
   }
   core.debug(`ref = '${result.ref}'`)
@@ -168,12 +185,19 @@ export async function getInputs(): Promise<IGitSourceSettings> {
     'TRUE'
   core.debug(`allow unsafe PR checkout = ${result.allowUnsafePrCheckout}`)
 
-  unsafePrCheckoutHelper.assertSafePrCheckout({
-    qualifiedRepository,
-    ref: result.ref,
-    commit: result.commit,
-    allowUnsafePrCheckout: result.allowUnsafePrCheckout
-  })
+  // The default self-checkout (this repository with no explicit ref) always
+  // resolves to the trusted ref/commit GitHub set for the triggering event, so
+  // the fork-checkout guard only needs to run when the caller customized the
+  // repository or ref.
+  const isDefaultCheckout = isWorkflowRepository && !core.getInput('ref')
+  if (!isDefaultCheckout) {
+    unsafePrCheckoutHelper.assertSafePrCheckout({
+      qualifiedRepository,
+      ref: result.ref,
+      commit: result.commit,
+      allowUnsafePrCheckout: result.allowUnsafePrCheckout
+    })
+  }
 
   return result
 }