Skip to content
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

[@actions/attest] Fix bug with customized OIDC issuer #1823

Merged
merged 1 commit into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/attest/RELEASES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# @actions/attest Releases

### 1.4.2

- Fix bug in `buildSLSAProvenancePredicate`/`attestProvenance` when generating provenance statement for enterprise account using customized OIDC issuer value [#1823](https://github.com/actions/toolkit/pull/1823)
### 1.4.1

- Bump @actions/http-client from 2.2.1 to 2.2.3 [#1805](https://github.com/actions/toolkit/pull/1805)
Expand All @@ -8,7 +11,6 @@

- Add new `headers` parameter to the `attest` and `attestProvenance` functions [#1790](https://github.com/actions/toolkit/pull/1790)
- Update `buildSLSAProvenancePredicate`/`attestProvenance` to automatically derive default OIDC issuer URL from current execution context [#1796](https://github.com/actions/toolkit/pull/1796)

### 1.3.1

- Fix bug with proxy support when retrieving JWKS for OIDC issuer [#1776](https://github.com/actions/toolkit/pull/1776)
Expand Down
53 changes: 52 additions & 1 deletion packages/attest/__tests__/oidc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,55 @@ describe('getIDTokenClaims', () => {
})
})

describe('when ID token is valid (w/ enterprise slug)', () => {
const claims = {
iss: `${issuer}/foo-bar`,
aud: audience,
ref: 'ref',
sha: 'sha',
repository: 'repo',
event_name: 'push',
job_workflow_ref: 'job_workflow_ref',
workflow_ref: 'workflow',
repository_id: '1',
repository_owner_id: '1',
runner_environment: 'github-hosted',
run_id: '1',
run_attempt: '1'
}

beforeEach(async () => {
const jwt = await new jose.SignJWT(claims)
.setProtectedHeader({alg: 'PS256'})
.sign(key.privateKey)

nock(issuer).get(tokenPath).query({audience}).reply(200, {value: jwt})
})

it('returns the ID token claims', async () => {
const result = await getIDTokenClaims(issuer)
expect(result).toEqual(claims)
})
})

describe('when ID token is missing the "iss" claim', () => {
const claims = {
aud: audience
}

beforeEach(async () => {
const jwt = await new jose.SignJWT(claims)
.setProtectedHeader({alg: 'PS256'})
.sign(key.privateKey)

nock(issuer).get(tokenPath).query({audience}).reply(200, {value: jwt})
})

it('throws an error', async () => {
await expect(getIDTokenClaims(issuer)).rejects.toThrow(/missing "iss"/i)
})
})

describe('when ID token is missing required claims', () => {
const claims = {
iss: issuer,
Expand Down Expand Up @@ -99,7 +148,9 @@ describe('getIDTokenClaims', () => {
})

it('throws an error', async () => {
await expect(getIDTokenClaims(issuer)).rejects.toThrow(/unexpected "iss"/)
await expect(getIDTokenClaims(issuer)).rejects.toThrow(
/unexpected "iss"/i
)
})
})

Expand Down
4 changes: 2 additions & 2 deletions packages/attest/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/attest/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@actions/attest",
"version": "1.4.1",
"version": "1.4.2",
"description": "Actions attestation lib",
"keywords": [
"github",
Expand Down
13 changes: 11 additions & 2 deletions packages/attest/src/oidc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,19 @@ const decodeOIDCToken = async (
// Verify and decode token
const jwks = jose.createLocalJWKSet(await getJWKS(issuer))
const {payload} = await jose.jwtVerify(token, jwks, {
audience: OIDC_AUDIENCE,
issuer
audience: OIDC_AUDIENCE
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The jose library only checks for absolute matches, so we're going to remove the check of the iss claim and implement our own regex-based check.

})

if (!payload.iss) {
throw new Error('Missing "iss" claim')
}

// Check that the issuer STARTS WITH the expected issuer URL to account for
// the fact that the value may include an enterprise-specific slug
if (!payload.iss.startsWith(issuer)) {
throw new Error(`Unexpected "iss" claim: ${payload.iss}`)
}

return payload
}

Expand Down
Loading