Validate repository org-ownership before storage record creation (#328)

* check if the repository is owned by org before attempting storage record creation

Signed-off-by: Meredith Lancaster <[email protected]>

* linter

Signed-off-by: Meredith Lancaster <[email protected]>

* generate dist

Signed-off-by: Meredith Lancaster <[email protected]>

* add fixtures for repoOwnerIsOrg function

Signed-off-by: Meredith Lancaster <[email protected]>

* formatter

Signed-off-by: Meredith Lancaster <[email protected]>

* clean up fixtures

Signed-off-by: Meredith Lancaster <[email protected]>

* more clean up

Signed-off-by: Meredith Lancaster <[email protected]>

* fix function declaration

Signed-off-by: Meredith Lancaster <[email protected]>

* clean up fixtures

Signed-off-by: Meredith Lancaster <[email protected]>

* add test when repo is not owned by org

Signed-off-by: Meredith Lancaster <[email protected]>

* add more expect statements, clean up mock calls

Signed-off-by: Meredith Lancaster <[email protected]>

* formatter

Signed-off-by: Meredith Lancaster <[email protected]>

* add more spy expect statements

Signed-off-by: Meredith Lancaster <[email protected]>

---------

Signed-off-by: Meredith Lancaster <[email protected]>

__tests__/main.test.ts

@@ -10,6 +10,7 @@ import * as github from '@actions/github'
 import { mockFulcio, mockRekor, mockTSA } from '@sigstore/mock'
 import * as oci from '@sigstore/oci'
 import * as attest from '@actions/attest'
+import * as localAttest from '../src/attest'
 import fs from 'fs/promises'
 import nock from 'nock'
 import os from 'os'
@@ -29,7 +30,7 @@ const setFailedMock = jest.spyOn(core, 'setFailed')
 setFailedMock.mockImplementation(() => {})
 
 const summaryWriteMock = jest.spyOn(core.summary, 'write')
-summaryWriteMock.mockImplementation(async () => Promise.resolve(core.summary))
+summaryWriteMock.mockResolvedValue(core.summary)
 
 // Mock the action's main function
 const runMock = jest.spyOn(main, 'run')
@@ -230,6 +231,9 @@ describe('action', () => {
   describe('when the repository is public', () => {
     const getRegCredsSpy = jest.spyOn(oci, 'getRegistryCredentials')
     const attachArtifactSpy = jest.spyOn(oci, 'attachArtifactToImage')
+    const repoOwnerIsOrgSpy = jest.spyOn(localAttest, 'repoOwnerIsOrg')
+    const createStorageRecordSpy = jest.spyOn(attest, 'createStorageRecord')
+    const createAttestationSpy = jest.spyOn(localAttest, 'createAttestation')
 
     const inputs: main.RunInputs = {
       ...defaultInputs,
@@ -258,13 +262,12 @@ describe('action', () => {
         username: 'username',
         password: 'password'
       }))
-      attachArtifactSpy.mockImplementation(async () =>
+      attachArtifactSpy.mockResolvedValue({
-        Promise.resolve({
+        digest: 'sha256:123456',
-          digest: 'sha256:123456',
+        mediaType: 'application/vnd.cncf.notary.v2',
-          mediaType: 'application/vnd.cncf.notary.v2',
+        size: 123456
-          size: 123456
+      })
-        })
+      repoOwnerIsOrgSpy.mockResolvedValue(true)
-      )
     })
 
     it('invokes the action w/o error', async () => {
@@ -274,6 +277,9 @@ describe('action', () => {
       expect(setFailedMock).not.toHaveBeenCalled()
       expect(getRegCredsSpy).toHaveBeenCalledWith(subjectName)
       expect(attachArtifactSpy).toHaveBeenCalled()
+      expect(createAttestationSpy).toHaveBeenCalled()
+      expect(repoOwnerIsOrgSpy).toHaveBeenCalled()
+      expect(createStorageRecordSpy).toHaveBeenCalled()
       expect(warningMock).not.toHaveBeenCalled()
       expect(infoMock).toHaveBeenNthCalledWith(
         1,
@@ -338,7 +344,6 @@ describe('action', () => {
 
     it('catches error when storage record creation fails and continues', async () => {
       // Mock the createStorageRecord function and throw an error
-      const createStorageRecordSpy = jest.spyOn(attest, 'createStorageRecord')
       createStorageRecordSpy.mockRejectedValueOnce(
         new Error('Failed to persist storage record: Not Found')
       )
@@ -346,12 +351,50 @@ describe('action', () => {
       await main.run(inputs)
 
       expect(runMock).toHaveReturned()
+      expect(createAttestationSpy).toHaveBeenCalled()
+      expect(repoOwnerIsOrgSpy).toHaveBeenCalled()
+      expect(createStorageRecordSpy).toHaveBeenCalled()
       expect(setFailedMock).not.toHaveBeenCalled()
       expect(warningMock).toHaveBeenNthCalledWith(
         1,
         expect.stringMatching('Failed to create storage record')
       )
     })
+
+    it('does not create a storage record when the repo is owned by a user', async () => {
+      repoOwnerIsOrgSpy.mockResolvedValueOnce(false)
+
+      await main.run(inputs)
+
+      expect(runMock).toHaveReturned()
+      expect(setFailedMock).not.toHaveBeenCalled()
+      expect(getRegCredsSpy).toHaveBeenCalledWith(subjectName)
+      expect(attachArtifactSpy).toHaveBeenCalled()
+      expect(createAttestationSpy).toHaveBeenCalled()
+      expect(repoOwnerIsOrgSpy).toHaveBeenCalled()
+      expect(createStorageRecordSpy).not.toHaveBeenCalled()
+      expect(warningMock).not.toHaveBeenCalled()
+      expect(infoMock).toHaveBeenCalledWith(
+        expect.stringMatching(
+          `Attestation created for ${subjectName}@${subjectDigest}`
+        )
+      )
+      expect(infoMock).not.toHaveBeenCalledWith(
+        expect.stringMatching('Storage record created')
+      )
+      expect(infoMock).not.toHaveBeenCalledWith(
+        expect.stringMatching('Storage record IDs: 987654321')
+      )
+      expect(setOutputMock).toHaveBeenCalledWith(
+        'attestation-id',
+        expect.stringMatching(attestationID)
+      )
+      expect(setOutputMock).not.toHaveBeenCalledWith(
+        'storage-record-ids',
+        expect.stringMatching(storageRecordID.toString())
+      )
+      expect(setFailedMock).not.toHaveBeenCalled()
+    })
   })
 
   describe('when the subject count is greater than 1', () => {

dist/index.js

@@ -124055,11 +124055,12 @@ var __importStar = (this && this.__importStar) || (function () {
     };
 })();
 Object.defineProperty(exports, "__esModule", ({ value: true }));
-exports.createAttestation = void 0;
+exports.repoOwnerIsOrg = exports.createAttestation = void 0;
 const attest_1 = __nccwpck_require__(11485);
 const oci_1 = __nccwpck_require__(81057);
 const subject_1 = __nccwpck_require__(36303);
 const core = __importStar(__nccwpck_require__(37484));
+const github = __importStar(__nccwpck_require__(93228));
 const OCI_TIMEOUT = 30000;
 const OCI_RETRY = 3;
 const createAttestation = async (subjects, predicate, opts) => {
@@ -124095,6 +124096,14 @@ const createAttestation = async (subjects, predicate, opts) => {
         // attestation process if the token does not have the correct permissions.
         if (opts.createStorageRecord) {
             try {
+                const token = opts.githubToken;
+                const isOrg = await (0, exports.repoOwnerIsOrg)(token);
+                if (!isOrg) {
+                    // The Artifact Metadata Storage Record API is only available to
+                    // organizations. So if the repo owner is not an organization,
+                    // storage record creation should not be attempted.
+                    return result;
+                }
                 const registryUrl = getRegistryURL(subject.name);
                 const artifactOpts = {
                     name: subject.name,
@@ -124103,7 +124112,7 @@ const createAttestation = async (subjects, predicate, opts) => {
                 const packageRegistryOpts = {
                     registryUrl
                 };
-                const records = await (0, attest_1.createStorageRecord)(artifactOpts, packageRegistryOpts, opts.githubToken);
+                const records = await (0, attest_1.createStorageRecord)(artifactOpts, packageRegistryOpts, token);
                 if (!records || records.length === 0) {
                     core.warning('No storage records were created.');
                 }
@@ -124118,6 +124127,18 @@ const createAttestation = async (subjects, predicate, opts) => {
     return result;
 };
 exports.createAttestation = createAttestation;
+// Call the GET /repos/{owner}/{repo} endpoint to determine if the repo
+// owner is an organization. This is used to determine if storage
+// record creation should be attempted.
+const repoOwnerIsOrg = async (githubToken) => {
+    const octokit = github.getOctokit(githubToken);
+    const { data: repo } = await octokit.rest.repos.get({
+        owner: github.context.repo.owner,
+        repo: github.context.repo.repo
+    });
+    return repo.owner?.type === 'Organization';
+};
+exports.repoOwnerIsOrg = repoOwnerIsOrg;
 function getRegistryURL(subjectName) {
     let url;
     try {

src/attest.ts

@@ -8,6 +8,7 @@ import {
 import { attachArtifactToImage, getRegistryCredentials } from '@sigstore/oci'
 import { formatSubjectDigest } from './subject'
 import * as core from '@actions/core'
+import * as github from '@actions/github'
 
 const OCI_TIMEOUT = 30000
 const OCI_RETRY = 3
@@ -64,6 +65,15 @@ export const createAttestation = async (
     // attestation process if the token does not have the correct permissions.
     if (opts.createStorageRecord) {
       try {
+        const token = opts.githubToken
+        const isOrg = await repoOwnerIsOrg(token)
+        if (!isOrg) {
+          // The Artifact Metadata Storage Record API is only available to
+          // organizations. So if the repo owner is not an organization,
+          // storage record creation should not be attempted.
+          return result
+        }
+
         const registryUrl = getRegistryURL(subject.name)
         const artifactOpts = {
           name: subject.name,
@@ -75,7 +85,7 @@ export const createAttestation = async (
         const records = await createStorageRecord(
           artifactOpts,
           packageRegistryOpts,
-          opts.githubToken
+          token
         )
 
         if (!records || records.length === 0) {
@@ -95,6 +105,18 @@ export const createAttestation = async (
   return result
 }
 
+// Call the GET /repos/{owner}/{repo} endpoint to determine if the repo
+// owner is an organization. This is used to determine if storage
+// record creation should be attempted.
+export const repoOwnerIsOrg = async (githubToken: string): Promise<boolean> => {
+  const octokit = github.getOctokit(githubToken)
+  const { data: repo } = await octokit.rest.repos.get({
+    owner: github.context.repo.owner,
+    repo: github.context.repo.repo
+  })
+  return repo.owner?.type === 'Organization'
+}
+
 function getRegistryURL(subjectName: string): string {
   let url: URL