ESM Conversion

[bdehamer]

This pull request migrates the codebase and test suite to full ECMAScript Module (ESM) support, modernizes test mocking patterns, and updates dependencies to their latest major versions.

The most significant changes are the adoption of ESM for both source and tests, refactoring of Jest test mocks to use jest.unstable_mockModule, and updates to the Jest and dependency configuration to ensure compatibility with ESM. These changes improve maintainability, future-proof the project, and align with the latest best practices for Node.js and the GitHub Actions ecosystem.

[Copilot]

Pull request overview

This PR migrates the action’s source and Jest test suite to full ESM, updates dependencies to newer major versions, and modernizes test mocking to use jest.unstable_mockModule for ESM-compatible module mocking.

Changes:

• Switch TypeScript + package configuration to ESM (type: module, updated tsconfig/jest preset).
• Refactor filesystem usage across the action to fs/promises and async APIs.
• Update and refactor tests to ESM-friendly mocking/import patterns; bump core dependencies.

Reviewed changes

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

Show a summary per file

File	Description
tsconfig.json	Moves TS output to ESM-oriented settings (module: ESNext, moduleResolution: Bundler).
src/subject.ts	Converts sync FS operations to async and updates stream usage.
src/sbom.ts	Converts to fs/promises and adjusts missing-file behavior.
src/predicate.ts	Makes predicateFromInputs async and uses fs/promises APIs.
src/main.ts	Switches to async FS writes and async temp dir creation; updates imports accordingly.
src/attest.ts	Import ordering tweak (no behavioral change).
package.json	Sets type: module, updates Jest config for ESM, updates deps, adjusts test scripts.
package-lock.json	Lockfile updates reflecting dependency major bumps.
jest.setup.js	Converts setup to ESM (import { jest } from '@jest/globals').
dist/package.json	Marks dist output as ESM (type: module).
dist/606.index.js	Updates bundled chunk output to ESM export form.
tests/sbom.test.ts	Updates expectations for missing SBOM error behavior.
tests/provenance.test.ts	Refactors mocks to jest.unstable_mockModule + dynamic import.
tests/predicate.test.ts	Updates tests for async predicateFromInputs.
tests/main.test.ts	Refactors to ESM mocks/dynamic imports; adjusts assertions.
tests/index.test.ts	Refactors index test to ESM mocks/dynamic import.
tests/attest.test.ts	Refactors attest tests to ESM mocks/dynamic import.

Comments suppressed due to low confidence (2)

package.json:37

• Same portability concern as ci-test: NODE_OPTIONS='--experimental-vm-modules' is not cross-platform in npm scripts. Consider a platform-agnostic invocation (or cross-env) to avoid Windows/local dev breakage.

    "test": "NODE_OPTIONS='--experimental-vm-modules' jest",
    "all": "npm run format:write && npm run lint && npm run test && npm run package"

tests/main.test.ts:364

• This test claims to cover a storage-record creation failure, but it doesn’t simulate an error: it just returns an attestation without storageRecordIds (which is also a valid success path). To actually exercise the failure-handling behavior, either have the mocked createAttestation reject/throw and assert warning behavior, or rename the test to reflect the scenario being tested.

    it('catches error when storage record creation fails and continues', async () => {
      // Mock createAttestation to simulate storage record failure (but still succeed overall)
      createAttestationMock.mockResolvedValue({
        attestationID,
        certificate:
          '-----BEGIN CERTIFICATE-----\ntest\n-----END CERTIFICATE-----',
        tlogID: 'tlog-123',
        attestationDigest: 'sha256:123456',
        bundle: { mediaType: 'application/vnd.dev.sigstore.bundle.v0.3+json' }
        // No storageRecordIDs - simulates empty/failed storage record
      })

      await run(inputs)

      expect(createAttestationMock).toHaveBeenCalled()
      expect(setFailedMock).not.toHaveBeenCalled()
    })

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test name says "repo is owned by a user", but the GitHub mock in this describe still returns an Organization owner type and the assertions don’t validate any user-owned behavior (e.g., lack of storage-record-ids output). Either adjust the mock (repos.get -> User) and assert the expected side effects, or rename/remove the test to avoid misleading coverage.

This issue also appears on line 348 of the same file.

[Copilot]

parseSBOMFromPath reads the entire SBOM file before checking its size, which defeats the 16MB guard and can load very large files into memory. Consider stat-ing first (and optionally handling ENOENT to throw a stable "SBOM file not found" error) before calling readFile.

See below for a potential fix:

  let stats
  try {
    stats = await fs.stat(filePath)
  } catch (error) {
    const err = error as NodeJS.ErrnoException
    if (err.code === 'ENOENT') {
      throw new Error('SBOM file not found')
    }
    throw error
  }

  if (stats.size > MAX_SBOM_SIZE_BYTES) {
    throw new Error(
      `SBOM file exceeds maximum allowed size: ${MAX_SBOM_SIZE_BYTES} bytes`
    )
  }

  const fileContent = await fs.readFile(filePath, 'utf8')

[Copilot]

getSubjectFromPath does Promise.all(paths.map(stat)) before enforcing MAX_SUBJECT_COUNT. If a glob expands to a very large list, this will issue a large number of concurrent stat calls and can become slow or hit OS limits. Consider short-circuiting when paths.length is already over the maximum, and/or iterating with a concurrency limit while counting files.

See below for a potential fix:

  // Filter path list to just the files (not directories), enforcing the maximum
  const files: string[] = []
  for (const p of paths) {
    const stat = await fs.stat(p)
    if (stat.isFile()) {
      files.push(p)
      if (files.length > MAX_SUBJECT_COUNT) {
        throw new Error(
          `Too many subjects specified (${files.length}). The maximum number of subjects is ${MAX_SUBJECT_COUNT}.`
        )
      }
    }

[Copilot]

The /* istanbul ignore next */ before const stat = await fs.stat(...) looks misplaced; it will ignore coverage for the stat line rather than the size-check branch. If the intent is to ignore the oversized-file branch, keep a single istanbul ignore next immediately before the if (stat.size > ...).

Suggested change

/* istanbul ignore next */