Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
3 changes: 3 additions & 0 deletions .github/actions/file/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ inputs:
token:
description: "Token with fine-grained permission 'issues: write'"
required: true
base_url:
description: "Optional base URL for the GitHub API (for example, 'https://HOSTNAME/api/v3' for GitHub Enterprise Server)"
required: false
cached_filings:
description: "Cached filings from previous runs, as stringified JSON. Without this, duplicate issues may be filed."
required: false
Expand Down
3 changes: 3 additions & 0 deletions .github/actions/file/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,22 @@ export default async function () {
const findings: Finding[] = JSON.parse(core.getInput('findings', {required: true}))
const repoWithOwner = core.getInput('repository', {required: true})
const token = core.getInput('token', {required: true})
const baseUrl = core.getInput('base_url', {required: false}) || undefined
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit; would this not implicitly default to undefined if its not provided?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry, completely missed this in my Outlook notifications -- yeah, that's correct, not sure why Copilot broke from the convention set above. going to remove

const screenshotRepo = core.getInput('screenshot_repository', {required: false}) || repoWithOwner
const cachedFilings: (ResolvedFiling | RepeatedFiling)[] = JSON.parse(
core.getInput('cached_filings', {required: false}) || '[]',
)
const shouldOpenGroupedIssues = core.getBooleanInput('open_grouped_issues')
core.debug(`Input: 'findings: ${JSON.stringify(findings)}'`)
core.debug(`Input: 'repository: ${repoWithOwner}'`)
core.debug(`Input: 'base_url: ${baseUrl ?? '(default)'}'`)
core.debug(`Input: 'screenshot_repository: ${screenshotRepo}'`)
core.debug(`Input: 'cached_filings: ${JSON.stringify(cachedFilings)}'`)
core.debug(`Input: 'open_grouped_issues: ${shouldOpenGroupedIssues}'`)

const octokit = new OctokitWithThrottling({
auth: token,
baseUrl,
throttle: {
onRateLimit: (retryAfter, options, octokit, retryCount) => {
octokit.log.warn(`Request quota exhausted for request ${options.method} ${options.url}`)
Expand Down
92 changes: 92 additions & 0 deletions .github/actions/file/tests/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import {beforeEach, describe, expect, it, vi} from 'vitest'

const {octokitCtorMock, getInputMock, getBooleanInputMock} = vi.hoisted(() => ({
octokitCtorMock: vi.fn(),
getInputMock: vi.fn(),
getBooleanInputMock: vi.fn(),
}))

vi.mock('@actions/core', () => ({
getInput: getInputMock,
getBooleanInput: getBooleanInputMock,
info: vi.fn(),
debug: vi.fn(),
warning: vi.fn(),
setOutput: vi.fn(),
setFailed: vi.fn(),
}))

vi.mock('@octokit/core', () => ({
Octokit: {
plugin: vi.fn(() => octokitCtorMock),
},
}))

vi.mock('@octokit/plugin-throttling', () => ({
throttling: vi.fn(),
}))

describe('file action index', () => {
beforeEach(() => {
vi.resetModules()
vi.clearAllMocks()
})

it('passes baseUrl to Octokit when base_url input is provided', async () => {
getInputMock.mockImplementation((name: string) => {
switch (name) {
case 'findings':
return '[]'
case 'repository':
return 'org/repo'
case 'token':
return 'token'
case 'base_url':
return 'https://ghe.example.com/api/v3'
case 'cached_filings':
return '[]'
default:
return ''
}
})
getBooleanInputMock.mockReturnValue(false)

const {default: run} = await import('../src/index.ts')
await run()

expect(octokitCtorMock).toHaveBeenCalledWith(
expect.objectContaining({
auth: 'token',
baseUrl: 'https://ghe.example.com/api/v3',
}),
)
})

it('uses Octokit default API URL when base_url input is not provided', async () => {
getInputMock.mockImplementation((name: string) => {
switch (name) {
case 'findings':
return '[]'
case 'repository':
return 'org/repo'
case 'token':
return 'token'
case 'cached_filings':
return '[]'
default:
return ''
}
})
getBooleanInputMock.mockReturnValue(false)

const {default: run} = await import('../src/index.ts')
await run()

expect(octokitCtorMock).toHaveBeenCalledWith(
expect.objectContaining({
auth: 'token',
baseUrl: undefined,
}),
)
})
})
3 changes: 3 additions & 0 deletions .github/actions/fix/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ inputs:
token:
description: "Personal access token (PAT) with fine-grained permissions 'issues: write' and 'pull_requests: write'"
required: true
base_url:
description: "Optional base URL for the GitHub API (for example, 'https://HOSTNAME/api/v3' for GitHub Enterprise Server)"
required: false

outputs:
fixings:
Expand Down
3 changes: 3 additions & 0 deletions .github/actions/fix/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@ export default async function () {
const issues: IssueInput[] = JSON.parse(core.getInput('issues', {required: true}) || '[]')
const repoWithOwner = core.getInput('repository', {required: true})
const token = core.getInput('token', {required: true})
const baseUrl = core.getInput('base_url', {required: false}) || undefined
core.debug(`Input: 'issues: ${JSON.stringify(issues)}'`)
core.debug(`Input: 'repository: ${repoWithOwner}'`)
core.debug(`Input: 'base_url: ${baseUrl ?? '(default)'}'`)

const octokit = new OctokitWithThrottling({
auth: token,
baseUrl,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need the key here, even just to be consistent? (I am also overthinking the quotes oops)

Suggested change
baseUrl,
"baseUrl": baseUrl,

Copy link
Copy Markdown
Contributor

@abdulahmad307 abdulahmad307 Mar 26, 2026

Choose a reason for hiding this comment

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

I might be misunderstanding the question; are you asking 'should we always include the key 'baseUrl' even if a value is not provided for it via inputs'?

assuming that's your question (and assuming you dont already know the following 😅 - you can ignore me if you already know); assigning baseUrl to the options object that OctokitWithThrottling accepts will automatically assign both key and value to that object (even if the value is undefined).

const someVar = undefined
const object = {
  someVar 
}

// will result in an object that looks like this:
{
  someVar: undefined
}

so line 24 looks right to me (unless you were referring to something else that I'm not seeing)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

^agreed with Abdul -- we get a neat little shorthand/syntactic sugar if the key and value share a name (it's an ES6 thing)

throttle: {
onRateLimit: (retryAfter, options, octokit, retryCount) => {
octokit.log.warn(`Request quota exhausted for request ${options.method} ${options.url}`)
Expand Down
84 changes: 84 additions & 0 deletions .github/actions/fix/tests/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import {beforeEach, describe, expect, it, vi} from 'vitest'

const {octokitCtorMock, getInputMock} = vi.hoisted(() => ({
octokitCtorMock: vi.fn(),
getInputMock: vi.fn(),
}))

vi.mock('@actions/core', () => ({
getInput: getInputMock,
info: vi.fn(),
debug: vi.fn(),
warning: vi.fn(),
setOutput: vi.fn(),
setFailed: vi.fn(),
}))

vi.mock('@octokit/core', () => ({
Octokit: {
plugin: vi.fn(() => octokitCtorMock),
},
}))

vi.mock('@octokit/plugin-throttling', () => ({
throttling: vi.fn(),
}))

describe('fix action index', () => {
beforeEach(() => {
vi.resetModules()
vi.clearAllMocks()
})

it('passes baseUrl to Octokit when base_url input is provided', async () => {
getInputMock.mockImplementation((name: string) => {
switch (name) {
case 'issues':
return '[]'
case 'repository':
return 'org/repo'
case 'token':
return 'token'
case 'base_url':
return 'https://ghe.example.com/api/v3'
default:
return ''
}
})

const {default: run} = await import('../src/index.ts')
await run()

expect(octokitCtorMock).toHaveBeenCalledWith(
expect.objectContaining({
auth: 'token',
baseUrl: 'https://ghe.example.com/api/v3',
}),
)
})

it('uses Octokit default API URL when base_url input is not provided', async () => {
getInputMock.mockImplementation((name: string) => {
switch (name) {
case 'issues':
return '[]'
case 'repository':
return 'org/repo'
case 'token':
return 'token'
default:
return ''
}
})

const {default: run} = await import('../src/index.ts')
await run()

expect(octokitCtorMock).toHaveBeenCalledWith(
expect.objectContaining({
auth: 'token',
baseUrl: undefined,
}),
)
})
})
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ jobs:
REPLACE_THIS
repository: REPLACE_THIS/REPLACE_THIS # Provide a repository name-with-owner (in the format "primer/primer-docs"). This is where issues will be filed and where Copilot will open PRs; more information below.
token: ${{ secrets.GH_TOKEN }} # This token must have write access to the repo above (contents, issues, and PRs); more information below. Note: GitHub Actions' GITHUB_TOKEN cannot be used here.
# base_url: https://HOSTNAME/api/v3 # Optional: GitHub API base URL (required for GitHub Enterprise Server)
cache_key: REPLACE_THIS # Provide a filename that will be used when caching results. We recommend including the name or domain of the site being scanned.
# login_url: # Optional: URL of the login page if authentication is required
# username: # Optional: Username for authentication
Expand Down Expand Up @@ -117,6 +118,7 @@ Trigger the workflow manually or automatically based on your configuration. The
| `urls` | Yes | Newline-delimited list of URLs to scan | `https://primer.style`<br>`https://primer.style/octicons` |
| `repository` | Yes | Repository (with owner) for issues and PRs | `primer/primer-docs` |
| `token` | Yes | PAT with write permissions (see above) | `${{ secrets.GH_TOKEN }}` |
| `base_url` | No | GitHub API base URL used by Octokit. Set this for GitHub Enterprise Server (format: `https://HOSTNAME/api/v3`). Defaults to `https://api.github.com` | `https://ghe.example.com/api/v3` |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here - let's move under cache_key since it's not required and cache_key is.

| `cache_key` | Yes | Key for caching results across runs<br>Allowed: `A-Za-z0-9._/-` | `cached_results-primer.style-main.json` |
| `login_url` | No | If scanned pages require authentication, the URL of the login page | `https://github.com/login` |
| `username` | No | If scanned pages require authentication, the username to use for login | `some-user` |
Expand Down
5 changes: 5 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ inputs:
token:
description: "Personal access token (PAT) with fine-grained permissions 'contents: write', 'issues: write', and 'pull_requests: write'"
required: true
base_url:
description: "Optional base URL for the GitHub API (for example, 'https://HOSTNAME/api/v3' for GitHub Enterprise Server)"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: "Optional base URL for the GitHub API (for example, 'https://HOSTNAME/api/v3' for GitHub Enterprise Server)"
description: "Optional base URL to pass into Octokit for the GitHub API (for example, `https://YOUR_HOSTNAME/api/v3` for GitHub Enterprise Server)"

required: false
cache_key:
description: 'Key for caching results across runs'
required: true
Expand Down Expand Up @@ -113,6 +116,7 @@ runs:
findings: ${{ steps.find.outputs.findings }}
repository: ${{ inputs.repository }}
token: ${{ inputs.token }}
base_url: ${{ inputs.base_url }}
cached_filings: ${{ steps.normalize_cache.outputs.value }}
screenshot_repository: ${{ github.repository }}
open_grouped_issues: ${{ inputs.open_grouped_issues }}
Expand All @@ -132,6 +136,7 @@ runs:
issues: ${{ steps.get_issues_from_filings.outputs.issues }}
repository: ${{ inputs.repository }}
token: ${{ inputs.token }}
base_url: ${{ inputs.base_url }}
- name: Set results output
id: results
uses: actions/github-script@v8
Expand Down
Loading