Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Linux-only GitHub Action "setup-adc" (Node24) to install the ADC CLI, including TypeScript source, build/config files, action metadata, README, editor/formatter configs, .gitignore updates, and a GitHub Actions test workflow exercising latest, pinned (0.24.0), and ARM64 installs. ChangesSetup ADC Action
Sequence DiagramsequenceDiagram
actor User as Workflow User
participant Actions as GitHub Actions
participant ActionCode as setup-adc (node24)
participant Releases as GitHub Releases API
participant ToolCache as `@actions/tool-cache`
participant Runner as Runner FS / PATH
User->>Actions: Trigger workflow using setup-adc
Actions->>ActionCode: Invoke action with inputs (version, token)
ActionCode->>Releases: Request latest release (if version='latest') or use specified tag
Releases-->>ActionCode: Return tag/version
ActionCode->>ActionCode: Validate OS is Linux and resolve arch
ActionCode->>ToolCache: Check for cached ADC version+arch
alt cached
ToolCache-->>ActionCode: Return cached tool path
else not cached
ActionCode->>Releases: Download release tarball for version+arch
Releases-->>ActionCode: Tarball
ActionCode->>ActionCode: Extract and prepare binary
ActionCode->>ToolCache: Cache extracted tool
ToolCache-->>ActionCode: Cache path
end
ActionCode->>Runner: Add ADC path to PATH
ActionCode-->>Actions: Set output `adc-version`
Actions-->>User: Continue workflow with ADC available
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Pull request overview
Introduces an initial TypeScript-based GitHub Action to install the API7 ADC CLI on Linux runners, including version resolution via GitHub Releases and tool-cache integration.
Changes:
- Adds the action implementation (
src/main.ts) to resolve/download/extract/cache ADC and exposeadc-version. - Adds action metadata (
action.yml) plus basic documentation (README.md). - Adds TypeScript/tooling configuration and a CI workflow intended to validate installation.
Reviewed changes
Copilot reviewed 7 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | TypeScript compiler configuration for building the action. |
| src/main.ts | Core action logic: inputs, version resolution, download/cache, PATH/outputs. |
| README.md | Usage documentation, inputs/outputs. |
| pnpm-lock.yaml | Dependency lockfile for pnpm. |
| package.json | Dependencies and build/format scripts (ncc bundling). |
| action.yml | Action metadata: inputs/outputs and Node20 entrypoint. |
| .vscode/settings.json | Editor formatting setting. |
| .prettierrc | Prettier configuration (including import sorting). |
| .gitignore | Ignores Node artifacts and build output. |
| .github/workflows/test.yml | CI workflow to exercise the action. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
README.md (1)
12-12: ⚡ Quick winPin the action reference to a commit SHA for security and reproducibility.
Using
@mainallows the action to change on every workflow run. Pin to a full commit SHA (e.g.,uses: api7/setup-adc@<commit-SHA>) to ensure immutable, reproducible behavior. GitHub's security documentation recommends commit SHAs over branch references and even tags, since SHA references cannot be moved or deleted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 12, Replace the unstable branch ref for the GitHub Action usage (the line containing "uses: api7/setup-adc@main") with an immutable commit SHA reference; find the "uses: api7/setup-adc@main" entry and change it to "uses: api7/setup-adc@<commit-SHA>" where <commit-SHA> is the full 40-character commit hash from the action repository to ensure reproducible, secure workflow runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Line 40: The workflow job uses an invalid runner label "ubuntu-latest-arm" in
the runs-on field; update that value to a supported GitHub-hosted ARM runner
(e.g., "ubuntu-24.04-arm") by changing the runs-on entry in the workflow where
the job defines runs-on to a valid ARM label so the job can be scheduled.
In @.gitignore:
- Line 2: The repository currently ignores the built action output (dist/) which
breaks the JavaScript GitHub Action runtime because action.yml references
dist/index.js; remove or stop ignoring dist/ in .gitignore so the built
artifacts are committed (or alternatively update action.yml to point to a
committed source and add a build step), specifically ensure dist/ is not listed
in .gitignore and that the artifact referenced by action.yml (dist/index.js) is
present in the repo.
In `@src/main.ts`:
- Around line 43-48: The code currently calls tc.downloadTool(downloadUrl) and
then tc.extractTar(downloadPath) without verifying the archive; modify the flow
in main.ts to verify integrity of downloadPath before calling extractTar by
fetching or receiving an expected checksum or signature (from release metadata,
a .sha256 file, or a signed release asset), computing the archive digest (e.g.,
SHA256) for downloadPath and comparing it to the expected value (or verifying a
detached signature), and if the check fails throw/log an error and abort; update
the logic around the downloadPath and extractedPath variables and ensure
verification occurs after downloadTool and before extractTar so extractTar is
only called on verified archives.
- Around line 87-113: The code currently lowercases the trimmed input (variable
versionSpec → normalized) which loses case in explicit version strings; change
the logic to only perform case-insensitive comparison for the "latest" check
while preserving the original trimmed input for explicit versions: compute a
trimmed variable (e.g. trimmed = versionSpec.trim()), check if trimmed === '' ||
trimmed.toLowerCase() === 'latest' to resolve GitHub latest release, otherwise
use the trimmed value (do not lowercase) and then strip a leading 'v' if present
to form the final version; update references to normalized/version accordingly
so prerelease/build identifiers keep their original case.
---
Nitpick comments:
In `@README.md`:
- Line 12: Replace the unstable branch ref for the GitHub Action usage (the line
containing "uses: api7/setup-adc@main") with an immutable commit SHA reference;
find the "uses: api7/setup-adc@main" entry and change it to "uses:
api7/setup-adc@<commit-SHA>" where <commit-SHA> is the full 40-character commit
hash from the action repository to ensure reproducible, secure workflow runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 68667f3d-7745-4444-8850-77f9b5d4ab7b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/workflows/test.yml.gitignore.prettierrc.vscode/settings.jsonREADME.mdaction.ymlpackage.jsonsrc/main.tstsconfig.json
| const downloadPath = await tc.downloadTool(downloadUrl); | ||
| core.info(`Downloaded to ${downloadPath}`); | ||
|
|
||
| // ── Extract ───────────────────────────────────────────── | ||
| const extractedPath = await tc.extractTar(downloadPath); | ||
| core.info(`Extracted to ${extractedPath}`); |
There was a problem hiding this comment.
Verify archive integrity before extraction.
Lines 43–48 download and extract a release tarball without checksum/signature verification. That weakens supply-chain security for every workflow using this action.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.ts` around lines 43 - 48, The code currently calls
tc.downloadTool(downloadUrl) and then tc.extractTar(downloadPath) without
verifying the archive; modify the flow in main.ts to verify integrity of
downloadPath before calling extractTar by fetching or receiving an expected
checksum or signature (from release metadata, a .sha256 file, or a signed
release asset), computing the archive digest (e.g., SHA256) for downloadPath and
comparing it to the expected value (or verifying a detached signature), and if
the check fails throw/log an error and abort; update the logic around the
downloadPath and extractedPath variables and ensure verification occurs after
downloadTool and before extractTar so extractTar is only called on verified
archives.
There was a problem hiding this comment.
This requires improving the release workflow for upstream projects so that they publish SHA-256 digest files and enable immutable releases.
Please create an issue for this.
There was a problem hiding this comment.
🧩 Analysis chain
@bzp2010, I've created a GitHub issue to track this. The issue captures the upstream requirements (SHA-256 digest publication and immutable releases) as prerequisites for adding checksum verification to the action, and links back to this PR and review comment. Once the upstream ADC release workflow is updated, the action can be amended accordingly.
🏁 Script executed:
gh issue create \
--repo api7/setup-adc \
--title "feat: verify archive integrity before extraction (SHA-256 checksum support)" \
--body "## Summary
Currently, \`src/main.ts\` downloads a release tarball via \`tc.downloadTool()\` and immediately extracts it via \`tc.extractTar()\` without verifying the archive's integrity. This weakens supply-chain security for every workflow using this action.
## Background
To properly implement checksum verification in the \`setup-adc\` action, the upstream ADC release workflow must first be enhanced to:
1. **Publish SHA-256 digest files** alongside each release asset (e.g., \`adc_linux_amd64.tar.gz.sha256\`).
2. **Enable immutable releases** so that release assets cannot be silently replaced after publication.
Once the upstream project provides these artifacts, the action can be updated to:
- Fetch the expected checksum from the release metadata or a \`.sha256\` sidecar file.
- Compute the SHA-256 digest of the downloaded archive.
- Compare the computed digest to the expected value and abort extraction if they do not match.
## References
- PR: https://github.com/api7/setup-adc/pull/1
- Review comment: https://github.com/api7/setup-adc/pull/1#discussion_r3193043662
/cc `@bzp2010`" \
--assignee bzp2010
Length of output: 43
@bzp2010, the issue has been created and assigned to you: #2
It documents the upstream prerequisites (publishing SHA-256 digest files and enabling immutable releases) needed before checksum verification can be added to this action.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 11 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 35-36: The "Verify version" step currently pipes adc --version
into grep 0.24.0 which treats dots as regex wildcards; change that to use
fixed-string matching (e.g., use grep --fixed-strings or grep -F with the
literal "0.24.0") so the command in the Verify version step (the line containing
`adc --version | grep 0.24.0`) only matches the exact literal version string and
avoids regex false positives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54522256-f600-4521-a11d-78d35c1b06da
⛔ Files ignored due to path filters (1)
dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (3)
.github/workflows/test.yml.prettierignoresrc/main.ts
✅ Files skipped from review due to trivial changes (1)
- .prettierignore
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 12 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 28-33: Update the README inputs table to include the missing token
input defined in action.yml: add a new row for `token` with a brief description
like "GitHub token used to fetch the latest release from the GitHub API", mark
it as not required (No) and set the Default to `${{ github.token }}` so it
appears alongside the existing `version` input in the Inputs table; ensure the
column order and formatting match the existing table for `version`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| ## Inputs | ||
|
|
||
| | Input | Description | Required | Default | | ||
| | --------- | --------------------------------------------------- | -------- | -------- | | ||
| | `version` | Version of ADC to install (e.g. `0.25.0`, `latest`) | No | `latest` | | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether action.yml exposes a `token` input and whether it is required.
fd -i '^action\.yml$' -X sed -n '1,220p'Repository: api7/setup-adc
Length of output: 680
Add token input to the inputs table in README.
The action.yml file defines a token input that is not documented in the README's inputs table (lines 28-33). The token input is optional and defaults to ${{ github.token }}, used for fetching the latest release from the GitHub API. Update the README to document this input alongside version to prevent integration confusion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 28 - 33, Update the README inputs table to include
the missing token input defined in action.yml: add a new row for `token` with a
brief description like "GitHub token used to fetch the latest release from the
GitHub API", mark it as not required (No) and set the Default to `${{
github.token }}` so it appears alongside the existing `version` input in the
Inputs table; ensure the column order and formatting match the existing table
for `version`.
Summary by CodeRabbit
New Features
Tests
Documentation
Style
Chores