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:
WalkthroughAdds four GitHub Actions workflows:
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/bb-fork-forge-test.yml (1)
86-97: Pin the Foundry checkout to an immutable ref.Cloning
feat/better-json-filedirectly makes this workflow non-reproducible: a later push or force-push to that branch can change CI behavior for the same repo commit. Pin a commit SHA (or surface the ref as an input) and let the existing hash step key the cache from that fixed checkout instead. The same clone block in.github/workflows/bb-forge-test.ymlshould stay in sync. GitHub recommends immutable refs for workflow dependencies for exactly this stability reason. (docs.github.com)Suggested change
- name: Clone Foundry repository working-directory: . run: | if [ ! -d "foundry" ]; then - git clone https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file + git clone https://github.com/BuildBearLabs/foundry.git foundry + git -C foundry checkout <pinned-foundry-sha> fi shell: bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-fork-forge-test.yml around lines 86 - 97, Replace the branch checkout with an immutable ref: update the "Clone Foundry repository" step to accept or compute a fixed commit SHA (rather than cloning the branch name) and then checkout that SHA in the cloned repo; tie the "Set Foundry commit hash" step to that same SHA so the cache key is stable. Concretely, expose a workflow input (or environment variable) like foundry_ref/FOUNDARY_SHA, change the clone step referenced by "Clone Foundry repository" to clone then git -C foundry fetch origin and git -C foundry checkout $FOUNDARY_SHA (or fetch+reset --hard $FOUNDARY_SHA), and ensure the "Set Foundry commit hash" step uses that same variable/value; apply the identical change to the other workflow file so both clones are pinned to the immutable commit SHA.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/bb-ci-flow.yml:
- Around line 10-26: The workflow's paths filter omits downstream test workflows
so changes to those files won't trigger this CI; update the pull_request and
push paths arrays in .github/workflows/bb-ci-flow.yml (the pull_request and push
-> paths entries) to also include ".github/workflows/bb-ci-module.yml",
".github/workflows/bb-forge-test.yml", and
".github/workflows/bb-fork-forge-test.yml" so changes to the nested test
workflows will trigger the CI entrypoint.
In @.github/workflows/bb-ci-module.yml:
- Around line 53-67: The job "test-fork" currently always invokes the reusable
workflow that runs forge with a secret fork URL (in bb-fork-forge-test.yml)
which will fail for fork/PR contexts without secrets; add an if guard on the
"test-fork" job to skip it for pull_request runs originating from forks or
Dependabot (e.g., use a condition combining github.event_name == 'pull_request'
and github.event.pull_request.head.repo.fork or github.actor ==
'dependabot[bot]') so the job only runs when secrets are available; update the
job "test-fork" in .github/workflows/bb-ci-module.yml to include this
if-condition.
In @.github/workflows/bb-forge-test.yml:
- Around line 35-38: The workflow declares the input fuzz-seed but never uses
it; either remove the unused input or wire it into the job by adding the
weekly-seed step before running forge test. To fix, update the reusable workflow
to check inputs.fuzz-seed and, when true, run the existing weekly-seed step (or
equivalent deterministic-seed step) prior to the step that executes `forge
test`; reference the `fuzz-seed` input name and the `forge test` step to ensure
the seed is applied in time, or alternatively delete the `fuzz-seed` input
definition to keep the API consistent.
---
Nitpick comments:
In @.github/workflows/bb-fork-forge-test.yml:
- Around line 86-97: Replace the branch checkout with an immutable ref: update
the "Clone Foundry repository" step to accept or compute a fixed commit SHA
(rather than cloning the branch name) and then checkout that SHA in the cloned
repo; tie the "Set Foundry commit hash" step to that same SHA so the cache key
is stable. Concretely, expose a workflow input (or environment variable) like
foundry_ref/FOUNDARY_SHA, change the clone step referenced by "Clone Foundry
repository" to clone then git -C foundry fetch origin and git -C foundry
checkout $FOUNDARY_SHA (or fetch+reset --hard $FOUNDARY_SHA), and ensure the
"Set Foundry commit hash" step uses that same variable/value; apply the
identical change to the other workflow file so both clones are pinned to the
immutable commit SHA.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fb8d391-9151-4bd2-9760-dbbe84c2a0f7
📒 Files selected for processing (4)
.github/workflows/bb-ci-flow.yml.github/workflows/bb-ci-module.yml.github/workflows/bb-forge-test.yml.github/workflows/bb-fork-forge-test.yml
| test-fork: | ||
| needs: ["build", "check"] | ||
| uses: "./.github/workflows/bb-fork-forge-test.yml" | ||
| secrets: inherit | ||
| with: | ||
| cache-path: | | ||
| ${{ inputs.module }}/cache | ||
| ${{ inputs.module }}/node_modules | ||
| ${{ inputs.module }}/out | ||
| ${{ inputs.module }}/out-optimized | ||
| foundry-fuzz-runs: 20 | ||
| foundry-profile: "test-optimized" | ||
| match-path: "tests/fork/**/*.sol" | ||
| name: "Fork tests" | ||
| working-directory: ${{ inputs.module }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find .github/workflows -name "*.yml" -o -name "*.yaml" | head -20Repository: sablier-labs/evm-monorepo
Length of output: 438
🏁 Script executed:
cat -n .github/workflows/bb-fork-forge-test.ymlRepository: sablier-labs/evm-monorepo
Length of output: 6396
🏁 Script executed:
cat -n .github/workflows/bb-ci-module.yml | sed -n '53,67p'Repository: sablier-labs/evm-monorepo
Length of output: 654
🌐 Web query:
GitHub Actions secrets fork pull request default behavior
💡 Result:
By default, GitHub Actions workflows triggered by pull requests from forked repositories (using the pull_request event) do not have access to repository, organization, or environment secrets (except the limited GITHUB_TOKEN with read-only permissions). This is a security measure to prevent malicious actors from exfiltrating secrets via modified workflows in the fork. This applies to both public and private repositories. Workflows run in the context of the fork for pull_request events, blocking secrets access. To enable secrets access: - For private repositories only: In repository Settings > Actions > General > Fork pull request workflows from private repositories, enable "Send secrets to workflows from pull requests". This makes secrets available but increases risk, so combine with "Require approval for fork pull request workflows". - Alternative (risky): Use pull_request_target event, which runs in base repo context with secrets, but requires careful handling to avoid executing untrusted fork code. Official recommendation: Avoid secrets in fork PR workflows or use OIDC/temporary credentials.
Citations:
- 1: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository
- 2: https://docs.github.com/actions/security-guides/using-secrets-in-github-actions
- 3: https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions
- 4: https://docs.github.com/en/code-security/getting-started/understanding-github-secret-types
🌐 Web query:
GitHub Actions if condition on job calling reusable workflow
💡 Result:
Yes, you can use an if condition on a job that calls a reusable workflow in GitHub Actions. The jobs.<job_id>.if expression is evaluated at the job level in the caller workflow before deciding whether to execute the reusable workflow. If the condition is false, the job (and thus the reusable workflow call) is skipped. Example from official documentation and community usage: jobs: conditional-reusable: if: ${{ github.ref_type == 'tag' }} uses: ./.github/workflows/reusable.yml with: input1: value secrets: inherit Official GitHub Docs confirm jobs.<job_id>.if works for any job, including those using uses for reusable workflows (docs on using conditions to control job execution and reuse workflows). Stack Overflow examples (e.g., question 71580690) show this syntax, where issues were due to condition logic (e.g., github.ref_type value) rather than syntax. Note: Jobs calling reusable workflows cannot have additional steps after uses, and strategy.matrix is unsupported on such jobs, but if conditions are fully supported. Skipped jobs report as "success" status.
Citations:
- 1: https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows
- 2: https://docs.github.com/actions/using-jobs/using-conditions-to-control-job-execution
- 3: https://stackoverflow.com/questions/71580690/how-to-call-a-reusable-workflow-conditionally-at-the-job-level-in-github-actio
- 4: https://stackoverflow.com/questions/76758012/call-reusable-workflow-conditionally-from-a-job
- 5: Bug in IF conditions on inputs for reusable workflows actions/runner#1602
🏁 Script executed:
cat -n .github/workflows/bb-ci-flow.yml | head -100Repository: sablier-labs/evm-monorepo
Length of output: 1089
🌐 Web query:
GitHub Actions Dependabot pull request secrets access behavior
💡 Result:
GitHub Actions workflows triggered by Dependabot pull requests have restricted access to secrets for security reasons. By default, these workflows are treated like fork pull requests: they receive a read-only GITHUB_TOKEN and cannot access standard GitHub Actions repository secrets. Instead, only Dependabot-specific secrets (configured under repository/organization Dependabot secrets) are available to these workflows. Dependabot itself uses Dependabot secrets to access private registries when creating updates, referenced in dependabot.yml as ${{secrets.NAME}}. To enable secrets in Dependabot-triggered workflows: - Store required secrets as Dependabot secrets (not Actions secrets). - Reference them identically in workflows; they appear in the secrets context. - For workflows needing both Dependabot and other triggers, use conditional logic or duplicate secrets. GITHUB_TOKEN permissions are read-only by default but can be increased via the 'permissions' key in the workflow YAML. This behavior has been consistent since changes in 2021 to prevent "pwn requests" from compromised dependencies.
Citations:
- 1: https://docs.github.com/en/code-security/dependabot/troubleshooting-dependabot/troubleshooting-dependabot-on-github-actions
- 2: https://docs.github.com/code-security/dependabot/working-with-dependabot/configuring-access-to-private-registries-for-dependabot
- 3: https://docs.github.com/en/code-security/reference/supply-chain-security/troubleshoot-dependabot/troubleshooting-dependabot-on-github-actions
- 4: https://docs.github.com/en/code-security/reference/supply-chain-security/dependabot-on-actions
- 5: https://docs.github.com/en/code-security/dependabot/working-with-dependabot/configuring-access-to-private-registries-for-dependabot
- 6: https://docs.github.com/en/code-security/getting-started/understanding-github-secret-types
Gate test-fork for PR contexts that cannot receive secrets.
This job unconditionally runs forge test --fork-url "$MAINNET_RPC_URL" (line 157 of .github/workflows/bb-fork-forge-test.yml). GitHub does not pass Actions secrets to pull_request runs from forks or Dependabot by default—those PRs will fail here before the BuildBear step even starts. Jobs that call reusable workflows support if: guards, so this should be skipped in those contexts unless you provision dedicated Dependabot secrets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/bb-ci-module.yml around lines 53 - 67, The job "test-fork"
currently always invokes the reusable workflow that runs forge with a secret
fork URL (in bb-fork-forge-test.yml) which will fail for fork/PR contexts
without secrets; add an if guard on the "test-fork" job to skip it for
pull_request runs originating from forks or Dependabot (e.g., use a condition
combining github.event_name == 'pull_request' and
github.event.pull_request.head.repo.fork or github.actor == 'dependabot[bot]')
so the job only runs when secrets are available; update the job "test-fork" in
.github/workflows/bb-ci-module.yml to include this if-condition.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
.github/workflows/bb-forge-test.yml (1)
35-38:⚠️ Potential issue | 🟡 Minor
fuzz-seedinput is still a no-op.
fuzz-seedis declared (Line 35-Line 38) but never consumed beforeforge test(Line 144), so callers can set it without effect.Suggested fix
- name: "Restore the cached build" uses: "sablier-labs/gha-utils/.github/actions/evm-cache@main" with: cache-path: ${{ inputs.cache-path }} fail-on-cache-miss: true + - name: "Generate fuzz seed that changes weekly" + if: ${{ inputs.fuzz-seed }} + run: | + seed=$(echo $(($EPOCHSECONDS / 604800))) + echo "FOUNDRY_FUZZ_SEED=$seed" >> "$GITHUB_ENV" + - name: "Run the tests" id: run-tests working-directory: ${{ inputs.working-directory }} run: 'forge test --match-path "${{ inputs.match-path }}"'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-forge-test.yml around lines 35 - 38, The workflow defines an input named fuzz-seed but never uses it before running the forge test command; update the job step that runs forge test (the step invoking "forge test") to read the workflow input "fuzz-seed" (e.g., from inputs or env) and conditionally pass it to forge (for example add an environment variable or append a flag like --fuzz-seed or --seed <value> when inputs.fuzz-seed is true/non-empty) so the declared input actually affects the test invocation.
🧹 Nitpick comments (2)
.github/workflows/bb-forge-test.yml (1)
125-125: Useactions/cache/save@v4for version consistency.Line 125 uses
actions/cache/save@v3while restore usesactions/cache@v4(Line 93). Aligning major versions avoids dependency drift.Suggested fix
- - name: Save Foundry build cache + - name: Save Foundry build cache if: steps.cache-foundry.outputs.cache-hit != 'true' - uses: actions/cache/save@v3 + uses: actions/cache/save@v4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-forge-test.yml at line 125, The workflow uses actions/cache/save@v3 for saving the cache while the restore step uses actions/cache@v4, causing inconsistent major versions; update the save step (the uses: actions/cache/save@v3 entry) to actions/cache/save@v4 so both save and restore use the same major version (v4) to avoid dependency drift..github/workflows/bb-fork-forge-test.yml (1)
130-133: Useactions/cache/save@v4to match restore version.Line 132 still uses v3 while restore is v4. Aligning majors avoids drift and keeps behavior/support consistent.
♻️ Suggested one-line update
- uses: actions/cache/save@v3 + uses: actions/cache/save@v4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-fork-forge-test.yml around lines 130 - 133, Update the workflow step named "Save Foundry build cache" so its uses: reference matches the restore action major version; change uses: actions/cache/save@v3 to actions/cache/save@v4 to align with the restore version and avoid version drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/bb-forge-test.yml:
- Around line 64-70: Remove the job-level export of sensitive secrets
(API_KEY_INFURA and MAINNET_RPC_URL) from the env block and instead set those
secrets only in the specific step-level env entries that actually require them
(e.g., the steps that run Foundry/forge or need Mainnet access); ensure any
steps that clone or build external/untrusted code do not inherit those secrets
by keeping their env empty or explicitly unsetting them, and prefer passing
tokens via step inputs (with: or env:) only for trusted steps.
- Around line 79-84: The workflow clones the external Foundry repo using a
mutable branch ("git clone ... -b feat/better-json-file"), which undermines
reproducibility; change the clone step in the jobs that run in
.github/workflows/bb-forge-test.yml (and replicate in bb-fork-forge-test.yml) to
checkout a specific immutable commit SHA instead of the branch — e.g., obtain or
pass a pinned SHA variable earlier in the workflow (or fetch the branch and run
git checkout <COMMIT_SHA>) and replace the -b feat/better-json-file usage with
that commit SHA so the clone is deterministic and cache keys remain valid.
In @.github/workflows/bb-fork-forge-test.yml:
- Around line 59-60: The MAINNET_RPC_URL environment input is currently marked
required: false but fork tests always run with --fork-url (see use at the step
invoking the fork tests around the --fork-url flag), causing runtime failures;
update the workflow input for MAINNET_RPC_URL to be required (set required:
true) so the secret must be provided before running, and ensure any step
referencing MAINNET_RPC_URL (the fork test invocation) continues to use that
input.
- Around line 68-77: The job currently defines sensitive env vars
(API_KEY_INFURA, MAINNET_RPC_URL, ETHERSCAN_API_KEY, BUILDBEAR_BASE_URL) at the
job-level which exposes them to all steps; change to remove these from the
top-level env and instead add them only to the specific steps that require them
(e.g., the steps that perform forking, external repo clone/build, and any
Etherscan verification steps), and remove or avoid using broad "secrets:
inherit" where it grants secrets to unrelated steps; update the steps that need
those vars to include an env: block with the exact secret references and keep
other steps secret-free to minimize exposure.
- Line 143: Replace floating/mutable action refs with full 40-character commit
SHAs: change the uses entry
"sablier-labs/gha-utils/.github/actions/evm-cache@main" to the exact commit SHA
that corresponds to the desired main revision, and likewise replace the other
mutable tag reference "v1.7.4" with its 40-char commit SHA; update both uses
lines so they point to immutable commit SHAs to ensure reproducible builds and
supply-chain hardening.
- Around line 86-91: The workflow currently clones the Foundry repo using a
moving branch name ("feat/better-json-file"); replace that with an immutable
commit SHA by cloning and checking out the specific commit (i.e., use the repo
URL and then `git -C foundry checkout <COMMIT_SHA>` or clone the repo at the
tag/commit) so the workflow is deterministic; update the git clone command in
the "Clone Foundry repository" step (and the duplicate occurrence in the other
workflow step referenced) to fetch and checkout the chosen commit SHA instead of
the branch name "feat/better-json-file".
---
Duplicate comments:
In @.github/workflows/bb-forge-test.yml:
- Around line 35-38: The workflow defines an input named fuzz-seed but never
uses it before running the forge test command; update the job step that runs
forge test (the step invoking "forge test") to read the workflow input
"fuzz-seed" (e.g., from inputs or env) and conditionally pass it to forge (for
example add an environment variable or append a flag like --fuzz-seed or --seed
<value> when inputs.fuzz-seed is true/non-empty) so the declared input actually
affects the test invocation.
---
Nitpick comments:
In @.github/workflows/bb-forge-test.yml:
- Line 125: The workflow uses actions/cache/save@v3 for saving the cache while
the restore step uses actions/cache@v4, causing inconsistent major versions;
update the save step (the uses: actions/cache/save@v3 entry) to
actions/cache/save@v4 so both save and restore use the same major version (v4)
to avoid dependency drift.
In @.github/workflows/bb-fork-forge-test.yml:
- Around line 130-133: Update the workflow step named "Save Foundry build cache"
so its uses: reference matches the restore action major version; change uses:
actions/cache/save@v3 to actions/cache/save@v4 to align with the restore version
and avoid version drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0631a42-3a77-44f1-bfa2-b089df589efd
📒 Files selected for processing (4)
.github/workflows/bb-ci-flow.yml.github/workflows/bb-ci-module.yml.github/workflows/bb-forge-test.yml.github/workflows/bb-fork-forge-test.yml
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/bb-ci-flow.yml
- .github/workflows/bb-ci-module.yml
.github/workflows/bb-forge-test.yml
Outdated
| env: | ||
| API_KEY_INFURA: ${{ secrets.API_KEY_INFURA }} | ||
| FOUNDRY_INVARIANT_DEPTH: ${{ inputs.foundry-invariant-depth }} | ||
| FOUNDRY_INVARIANT_RUNS: ${{ inputs.foundry-invariant-runs }} | ||
| FOUNDRY_FUZZ_RUNS: ${{ inputs.foundry-fuzz-runs }} | ||
| FOUNDRY_PROFILE: ${{ inputs.foundry-profile }} | ||
| MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }} |
There was a problem hiding this comment.
Limit secret scope to trusted steps only.
At Line 65 and Line 70, secrets are exported at job scope, making them available to every step, including cloning/building external code (Line 83 and Line 117). That is a secret-exfiltration risk.
Suggested fix
forge-test:
env:
- API_KEY_INFURA: ${{ secrets.API_KEY_INFURA }}
FOUNDRY_INVARIANT_DEPTH: ${{ inputs.foundry-invariant-depth }}
FOUNDRY_INVARIANT_RUNS: ${{ inputs.foundry-invariant-runs }}
FOUNDRY_FUZZ_RUNS: ${{ inputs.foundry-fuzz-runs }}
FOUNDRY_PROFILE: ${{ inputs.foundry-profile }}
- MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }}
runs-on: "ubuntu-latest"
@@
- name: "Run the tests"
id: run-tests
working-directory: ${{ inputs.working-directory }}
+ env:
+ API_KEY_INFURA: ${{ secrets.API_KEY_INFURA }}
+ MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }}
run: 'forge test --match-path "${{ inputs.match-path }}"'
continue-on-error: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env: | |
| API_KEY_INFURA: ${{ secrets.API_KEY_INFURA }} | |
| FOUNDRY_INVARIANT_DEPTH: ${{ inputs.foundry-invariant-depth }} | |
| FOUNDRY_INVARIANT_RUNS: ${{ inputs.foundry-invariant-runs }} | |
| FOUNDRY_FUZZ_RUNS: ${{ inputs.foundry-fuzz-runs }} | |
| FOUNDRY_PROFILE: ${{ inputs.foundry-profile }} | |
| MAINNET_RPC_URL: ${{ secrets.MAINNET_RPC_URL }} | |
| env: | |
| FOUNDRY_INVARIANT_DEPTH: ${{ inputs.foundry-invariant-depth }} | |
| FOUNDRY_INVARIANT_RUNS: ${{ inputs.foundry-invariant-runs }} | |
| FOUNDRY_FUZZ_RUNS: ${{ inputs.foundry-fuzz-runs }} | |
| FOUNDRY_PROFILE: ${{ inputs.foundry-profile }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/bb-forge-test.yml around lines 64 - 70, Remove the
job-level export of sensitive secrets (API_KEY_INFURA and MAINNET_RPC_URL) from
the env block and instead set those secrets only in the specific step-level env
entries that actually require them (e.g., the steps that run Foundry/forge or
need Mainnet access); ensure any steps that clone or build external/untrusted
code do not inherit those secrets by keeping their env empty or explicitly
unsetting them, and prefer passing tokens via step inputs (with: or env:) only
for trusted steps.
| - name: Clone Foundry repository | ||
| working-directory: . | ||
| run: | | ||
| if [ ! -d "foundry" ]; then | ||
| git clone https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full workflow file to understand the context
cat -n .github/workflows/bb-forge-test.ymlRepository: sablier-labs/evm-monorepo
Length of output: 5829
🏁 Script executed:
# Check if there are other git clone operations in workflows that might provide context
rg 'git clone' .github/workflows/ -A 2 -B 2Repository: sablier-labs/evm-monorepo
Length of output: 812
🏁 Script executed:
# Search for any comments or documentation about Foundry setup
rg -i 'foundry' .github/workflows/ -B 2 -A 2Repository: sablier-labs/evm-monorepo
Length of output: 21057
🏁 Script executed:
# Check if there are other instances of branch-based cloning vs. commit-pinning
rg 'git.*-b\s' .github/workflows/Repository: sablier-labs/evm-monorepo
Length of output: 329
Pin external Foundry source to an immutable ref instead of mutable branch.
Cloning feat/better-json-file at line 83 makes the CI behavior dependent on a mutable branch reference. While the workflow captures the commit hash for cache invalidation, explicitly pinning to a commit SHA in the clone command itself would strengthen reproducibility and supply-chain guarantees.
The same issue exists in .github/workflows/bb-fork-forge-test.yml.
Suggested fix
- name: Clone Foundry repository
working-directory: .
run: |
if [ ! -d "foundry" ]; then
- git clone https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file
+ git clone https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file
+ git -C foundry checkout <PINNED_COMMIT_SHA>
fi
shell: bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/bb-forge-test.yml around lines 79 - 84, The workflow
clones the external Foundry repo using a mutable branch ("git clone ... -b
feat/better-json-file"), which undermines reproducibility; change the clone step
in the jobs that run in .github/workflows/bb-forge-test.yml (and replicate in
bb-fork-forge-test.yml) to checkout a specific immutable commit SHA instead of
the branch — e.g., obtain or pass a pinned SHA variable earlier in the workflow
(or fetch the branch and run git checkout <COMMIT_SHA>) and replace the -b
feat/better-json-file usage with that commit SHA so the clone is deterministic
and cache keys remain valid.
| MAINNET_RPC_URL: | ||
| required: false |
There was a problem hiding this comment.
MAINNET_RPC_URL should be required here.
Line 157 always runs fork tests with --fork-url, so optional secret config at Line 59 causes avoidable runtime failures.
✅ Suggested fix
secrets:
@@
MAINNET_RPC_URL:
- required: false
+ required: trueAlso applies to: 157-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/bb-fork-forge-test.yml around lines 59 - 60, The
MAINNET_RPC_URL environment input is currently marked required: false but fork
tests always run with --fork-url (see use at the step invoking the fork tests
around the --fork-url flag), causing runtime failures; update the workflow input
for MAINNET_RPC_URL to be required (set required: true) so the secret must be
provided before running, and ensure any step referencing MAINNET_RPC_URL (the
fork test invocation) continues to use that input.
| env: | ||
| API_KEY_INFURA: "${{ secrets.API_KEY_INFURA }}" | ||
| FOUNDRY_INVARIANT_DEPTH: ${{ inputs.foundry-invariant-depth }} | ||
| FOUNDRY_INVARIANT_RUNS: ${{ inputs.foundry-invariant-runs }} | ||
| FOUNDRY_FUZZ_RUNS: ${{ inputs.foundry-fuzz-runs }} | ||
| FOUNDRY_PROFILE: ${{ inputs.foundry-profile }} | ||
| MAINNET_RPC_URL: "${{ secrets.MAINNET_RPC_URL }}" | ||
| ETHERSCAN_API_KEY: "${{ secrets.ETHERSCAN_API_KEY }}" | ||
| BUILDBEAR_BASE_URL: "${{ secrets.BUILDBEAR_BASE_URL }}" | ||
| runs-on: "ubuntu-latest" |
There was a problem hiding this comment.
Scope secrets to only the steps that need them.
Line 68 puts sensitive values in job-wide env, while Lines 86-126 clone/build external code first. With secrets: inherit in .github/workflows/bb-ci-module.yml:53-67, this widens secret exposure unnecessarily.
🔐 Suggested hardening diff
jobs:
forge-test:
env:
- API_KEY_INFURA: "${{ secrets.API_KEY_INFURA }}"
FOUNDRY_INVARIANT_DEPTH: ${{ inputs.foundry-invariant-depth }}
FOUNDRY_INVARIANT_RUNS: ${{ inputs.foundry-invariant-runs }}
FOUNDRY_FUZZ_RUNS: ${{ inputs.foundry-fuzz-runs }}
FOUNDRY_PROFILE: ${{ inputs.foundry-profile }}
- MAINNET_RPC_URL: "${{ secrets.MAINNET_RPC_URL }}"
- ETHERSCAN_API_KEY: "${{ secrets.ETHERSCAN_API_KEY }}"
- BUILDBEAR_BASE_URL: "${{ secrets.BUILDBEAR_BASE_URL }}"
runs-on: "ubuntu-latest"
@@
- name: "Run the tests"
id: run-tests
+ env:
+ MAINNET_RPC_URL: "${{ secrets.MAINNET_RPC_URL }}"
+ API_KEY_INFURA: "${{ secrets.API_KEY_INFURA }}"
+ ETHERSCAN_API_KEY: "${{ secrets.ETHERSCAN_API_KEY }}"
working-directory: ${{ inputs.working-directory }}
run: forge test --fork-url "$MAINNET_RPC_URL" --match-path "${{ inputs.match-path }}"
continue-on-error: true
@@
- name: "Run BB Action CI"
uses: BuildBearLabs/buildbear_x_action@v1.7.4
+ env:
+ BUILDBEAR_BASE_URL: "${{ secrets.BUILDBEAR_BASE_URL }}"
with:
buildbear-api-key: ${{ secrets.BUILDBEAR_API_KEY }}
working-directory: ${{ inputs.working-directory }}Also applies to: 86-91, 108-126
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/bb-fork-forge-test.yml around lines 68 - 77, The job
currently defines sensitive env vars (API_KEY_INFURA, MAINNET_RPC_URL,
ETHERSCAN_API_KEY, BUILDBEAR_BASE_URL) at the job-level which exposes them to
all steps; change to remove these from the top-level env and instead add them
only to the specific steps that require them (e.g., the steps that perform
forking, external repo clone/build, and any Etherscan verification steps), and
remove or avoid using broad "secrets: inherit" where it grants secrets to
unrelated steps; update the steps that need those vars to include an env: block
with the exact secret references and keep other steps secret-free to minimize
exposure.
| - name: Clone Foundry repository | ||
| working-directory: . | ||
| run: | | ||
| if [ ! -d "foundry" ]; then | ||
| git clone https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify mutable branch-based clones in workflows
rg -n 'git clone .* -b ' .github/workflowsRepository: sablier-labs/evm-monorepo
Length of output: 335
🏁 Script executed:
# Check the exact content at the target lines to ensure context accuracy
cat -n .github/workflows/bb-fork-forge-test.yml | sed -n '86,91p'Repository: sablier-labs/evm-monorepo
Length of output: 336
Pin Foundry to an immutable commit instead of a moving branch.
Cloning feat/better-json-file branch (line 90) allows its target to change without notice, breaking CI reproducibility and introducing security risk. Pin to a specific commit SHA instead.
📌 Suggested pinning fix
- name: Clone Foundry repository
working-directory: .
run: |
if [ ! -d "foundry" ]; then
- git clone https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file
+ git clone https://github.com/BuildBearLabs/foundry.git foundry
+ git -C foundry checkout --detach <PINNED_COMMIT_SHA>
fi
shell: bashNote: The same pattern exists in .github/workflows/bb-forge-test.yml:83 and should be fixed consistently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/bb-fork-forge-test.yml around lines 86 - 91, The workflow
currently clones the Foundry repo using a moving branch name
("feat/better-json-file"); replace that with an immutable commit SHA by cloning
and checking out the specific commit (i.e., use the repo URL and then `git -C
foundry checkout <COMMIT_SHA>` or clone the repo at the tag/commit) so the
workflow is deterministic; update the git clone command in the "Clone Foundry
repository" step (and the duplicate occurrence in the other workflow step
referenced) to fetch and checkout the chosen commit SHA instead of the branch
name "feat/better-json-file".
| run: forge --version | ||
|
|
||
| - name: "Restore the cached build" | ||
| uses: "sablier-labs/gha-utils/.github/actions/evm-cache@main" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List workflow action refs not pinned to a full commit SHA
rg -nP 'uses:\s*[^ ]+@(?![0-9a-f]{40}\b)[^ ]+' .github/workflowsRepository: sablier-labs/evm-monorepo
Length of output: 2691
🏁 Script executed:
sed -n '135,170p' .github/workflows/bb-fork-forge-test.yml | cat -nRepository: sablier-labs/evm-monorepo
Length of output: 1658
Pin action references to full commit SHAs for supply-chain hardening and reproducible builds.
Line 143 uses @main (floating ref) and line 161 uses @v1.7.4 (mutable tag). Replace with 40-character commit SHAs.
Pinning pattern
- - name: "Restore the cached build"
- uses: "sablier-labs/gha-utils/.github/actions/evm-cache@main"
+ - name: "Restore the cached build"
+ uses: "sablier-labs/gha-utils/.github/actions/evm-cache@<40-char-commit-sha>"
@@
- - name: "Run BB Action CI"
- uses: BuildBearLabs/buildbear_x_action@v1.7.4
+ - name: "Run BB Action CI"
+ uses: BuildBearLabs/buildbear_x_action@<40-char-commit-sha-for-v1.7.4>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/bb-fork-forge-test.yml at line 143, Replace
floating/mutable action refs with full 40-character commit SHAs: change the uses
entry "sablier-labs/gha-utils/.github/actions/evm-cache@main" to the exact
commit SHA that corresponds to the desired main revision, and likewise replace
the other mutable tag reference "v1.7.4" with its 40-char commit SHA; update
both uses lines so they point to immutable commit SHAs to ensure reproducible
builds and supply-chain hardening.
There was a problem hiding this comment.
♻️ Duplicate comments (4)
.github/workflows/bb-forge-test.yml (3)
35-38:⚠️ Potential issue | 🟡 MinorRemove or wire up
fuzz-seed.
inputs.fuzz-seedis declared but never read, so callers can set it and still get the default deterministic seed. Either add the seed-generation step beforeforge testor drop the input from this reusable workflow API.💡 Suggested fix
- name: "Restore the cached build" uses: "sablier-labs/gha-utils/.github/actions/evm-cache@main" with: cache-path: ${{ inputs.cache-path }} fail-on-cache-miss: true + - name: "Generate fuzz seed that changes weekly" + if: ${{ inputs.fuzz-seed }} + run: | + seed=$(($EPOCHSECONDS / 604800)) + echo "FOUNDRY_FUZZ_SEED=$seed" >> "$GITHUB_ENV" + - name: "Run the tests" id: run-tests working-directory: ${{ inputs.working-directory }} run: 'forge test --match-path "${{ inputs.match-path }}"'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-forge-test.yml around lines 35 - 38, The workflow declares an unused input inputs.fuzz-seed; either wire it into the run sequence or remove it. To fix, locate the reusable workflow's inputs block where fuzz-seed is declared and either (A) add a step (e.g., a step before the step that runs "forge test") that reads inputs.fuzz-seed, generates or exports the seed value into an environment variable (used by the "forge test" step) or passes it as an argument to the "forge test" invocation, ensuring the "forge test" step consumes that seed; or (B) remove the fuzz-seed entry from the inputs to avoid exposing an unused API. Ensure you reference inputs.fuzz-seed in the step that runs "forge test" (or remove the input entirely) so callers' values are honored.
62-67:⚠️ Potential issue | 🔴 CriticalScope
ROUTEMESH_API_KEYto the trusted step that actually needs it.Line 67 currently exposes the secret to every step, including the external
git cloneandcargo buildpath. Move it out of the jobenvand pass it only to the step(s) that consume it.🔒 Suggested fix
forge-test: env: FOUNDRY_INVARIANT_DEPTH: ${{ inputs.foundry-invariant-depth }} FOUNDRY_INVARIANT_RUNS: ${{ inputs.foundry-invariant-runs }} FOUNDRY_FUZZ_RUNS: ${{ inputs.foundry-fuzz-runs }} FOUNDRY_PROFILE: ${{ inputs.foundry-profile }} - ROUTEMESH_API_KEY: ${{ secrets.ROUTEMESH_API_KEY }} runs-on: "ubuntu-latest" @@ - name: "Run the tests" id: run-tests working-directory: ${{ inputs.working-directory }} + env: + ROUTEMESH_API_KEY: ${{ secrets.ROUTEMESH_API_KEY }} run: 'forge test --match-path "${{ inputs.match-path }}"' continue-on-error: trueAlso applies to: 138-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-forge-test.yml around lines 62 - 67, ROUTEMESH_API_KEY is set at the job-level env which exposes the secret to all steps; remove ROUTEMESH_API_KEY from the job-level env block and instead add it to the env: of only the specific step(s) that consume it (the step(s) that call the RouteMesh API or reference ROUTEMESH_API_KEY). Locate the workflow job env block containing FOUNDRY_* vars and the ROUTEMESH_API_KEY entry and delete that key there, then locate the individual step(s) that need the secret and add an env: ROUTEMESH_API_KEY: ${{ secrets.ROUTEMESH_API_KEY }} to those step definitions so the secret is scoped to the trusted step(s).
76-81:⚠️ Potential issue | 🟠 MajorPin the custom Foundry ref and keep it aligned with the build job.
Line 80 clones
feat/better-json-fileat a moving HEAD, and callers such as.github/workflows/bb-ci-module.ymlLines 30-40 build the cached artifacts withfoundry-version: "v1.4.4"before restoring them here. That means build and test are currently using different Forge binaries, which is a reproducibility and compatibility risk for the restoredoutartifacts.💡 Suggested fix
Use a single pinned Foundry ref/version for both stages. A minimal shape for the clone step here is:
- name: Clone Foundry repository working-directory: . run: | if [ ! -d "foundry" ]; then - git clone https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file + git clone https://github.com/BuildBearLabs/foundry.git foundry + git -C foundry checkout "${{ inputs.foundry-ref }}" fi shell: bashThen thread the same pinned ref through
.github/workflows/bb-ci-module.ymlso the build and test jobs share one toolchain.Also applies to: 84-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-forge-test.yml around lines 76 - 81, The clone step "Clone Foundry repository" is using an unpinned branch (feat/better-json-file) causing toolchain mismatch with the build job that sets foundry-version: "v1.4.4"; change the clone to use a single pinned ref (e.g. an env/secret or workflow input like FOUNDRY_REF) and set it to the same value used by the build (foundry-version), then update all other clone/checkout occurrences (the block around lines 84-127) to reference FOUNDRY_REF so both build and test jobs use identical Foundry/Forge binaries..github/workflows/bb-ci-module.yml (1)
51-65:⚠️ Potential issue | 🟠 MajorSkip
test-forkwhen PR secrets are unavailable.This job always invokes
.github/workflows/bb-fork-forge-test.yml, which relies on secret-backed forking. On forkedpull_requests and Dependabot PRs, those secrets are not available, so this becomes a deterministic red CI unless you guard it here.💡 Suggested fix
test-fork: + if: ${{ github.event_name != 'pull_request' || (github.actor != 'dependabot[bot]' && !github.event.pull_request.head.repo.fork) }} needs: ["build", "check"] uses: "./.github/workflows/bb-fork-forge-test.yml" secrets: inherit🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-ci-module.yml around lines 51 - 65, The test-fork job always runs and invokes the forked-secret workflow (uses: "./.github/workflows/bb-fork-forge-test.yml"), which fails on forked PRs and Dependabot PRs when repository secrets are unavailable; fix by adding an if conditional on the test-fork job to only run when not a forked PR (e.g. require that github.event.pull_request is null OR github.repository_owner == github.event.pull_request.head.repo.owner) so the job is skipped for PRs coming from forks (and thus secretless runs), keeping the existing job name test-fork and its uses value unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/bb-ci-module.yml:
- Around line 51-65: The test-fork job always runs and invokes the forked-secret
workflow (uses: "./.github/workflows/bb-fork-forge-test.yml"), which fails on
forked PRs and Dependabot PRs when repository secrets are unavailable; fix by
adding an if conditional on the test-fork job to only run when not a forked PR
(e.g. require that github.event.pull_request is null OR github.repository_owner
== github.event.pull_request.head.repo.owner) so the job is skipped for PRs
coming from forks (and thus secretless runs), keeping the existing job name
test-fork and its uses value unchanged.
In @.github/workflows/bb-forge-test.yml:
- Around line 35-38: The workflow declares an unused input inputs.fuzz-seed;
either wire it into the run sequence or remove it. To fix, locate the reusable
workflow's inputs block where fuzz-seed is declared and either (A) add a step
(e.g., a step before the step that runs "forge test") that reads
inputs.fuzz-seed, generates or exports the seed value into an environment
variable (used by the "forge test" step) or passes it as an argument to the
"forge test" invocation, ensuring the "forge test" step consumes that seed; or
(B) remove the fuzz-seed entry from the inputs to avoid exposing an unused API.
Ensure you reference inputs.fuzz-seed in the step that runs "forge test" (or
remove the input entirely) so callers' values are honored.
- Around line 62-67: ROUTEMESH_API_KEY is set at the job-level env which exposes
the secret to all steps; remove ROUTEMESH_API_KEY from the job-level env block
and instead add it to the env: of only the specific step(s) that consume it (the
step(s) that call the RouteMesh API or reference ROUTEMESH_API_KEY). Locate the
workflow job env block containing FOUNDRY_* vars and the ROUTEMESH_API_KEY entry
and delete that key there, then locate the individual step(s) that need the
secret and add an env: ROUTEMESH_API_KEY: ${{ secrets.ROUTEMESH_API_KEY }} to
those step definitions so the secret is scoped to the trusted step(s).
- Around line 76-81: The clone step "Clone Foundry repository" is using an
unpinned branch (feat/better-json-file) causing toolchain mismatch with the
build job that sets foundry-version: "v1.4.4"; change the clone to use a single
pinned ref (e.g. an env/secret or workflow input like FOUNDRY_REF) and set it to
the same value used by the build (foundry-version), then update all other
clone/checkout occurrences (the block around lines 84-127) to reference
FOUNDRY_REF so both build and test jobs use identical Foundry/Forge binaries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2945041d-7e6a-499f-b05d-2d38a10f8680
📒 Files selected for processing (3)
.github/workflows/bb-ci-module.yml.github/workflows/bb-forge-test.yml.github/workflows/bb-fork-forge-test.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/bb-fork-forge-test.yml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
.github/workflows/bb-forge-test.yml (3)
76-81:⚠️ Potential issue | 🟠 MajorPin Foundry checkout to an immutable commit SHA.
The clone uses a mutable branch ref (
-b feat/better-json-fileat Line 80). This makes runs non-reproducible and increases supply-chain risk.Suggested fix
- name: Clone Foundry repository working-directory: . run: | if [ ! -d "foundry" ]; then - git clone https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file + git clone https://github.com/BuildBearLabs/foundry.git foundry + git -C foundry checkout <PINNED_COMMIT_SHA> fi shell: bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-forge-test.yml around lines 76 - 81, The workflow step "Clone Foundry repository" currently clones a mutable branch (-b feat/better-json-file); change it to pin an immutable commit SHA by cloning the repo and checking out (or resetting) to the specific commit SHA instead of relying on the branch name — update the git commands in that step so they fetch the repo and then run a deterministic checkout/reset to the identified commit SHA (refer to the "Clone Foundry repository" step and the git clone invocation when making the change).
62-68:⚠️ Potential issue | 🔴 CriticalScope
ROUTEMESH_API_KEYto trusted step-level env only.
ROUTEMESH_API_KEYis exported at job scope (Line 67), so it is available to all steps, including external clone/build steps (Line 76 onward). Restrict it to the test step that needs it.Suggested fix
forge-test: env: FOUNDRY_INVARIANT_DEPTH: ${{ inputs.foundry-invariant-depth }} FOUNDRY_INVARIANT_RUNS: ${{ inputs.foundry-invariant-runs }} FOUNDRY_FUZZ_RUNS: ${{ inputs.foundry-fuzz-runs }} FOUNDRY_PROFILE: ${{ inputs.foundry-profile }} - ROUTEMESH_API_KEY: ${{ secrets.ROUTEMESH_API_KEY }} @@ - name: "Run the tests" id: run-tests working-directory: ${{ inputs.working-directory }} + env: + ROUTEMESH_API_KEY: ${{ secrets.ROUTEMESH_API_KEY }} run: 'forge test --match-path "${{ inputs.match-path }}"' continue-on-error: trueAlso applies to: 76-81, 98-117, 138-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-forge-test.yml around lines 62 - 68, The ROUTEMESH_API_KEY is currently set at job-level env (env: ROUTEMESH_API_KEY) exposing it to all steps; remove ROUTEMESH_API_KEY from the job-level env block and instead add it to the specific step-level env for the test step that requires it (the step that runs the Foundry tests), ensuring only that step references secrets.ROUTEMESH_API_KEY; apply the same change for the other occurrences called out (the other step env blocks referenced in the review) so the secret is only present in the minimal step scope.
35-38:⚠️ Potential issue | 🟡 Minor
fuzz-seedinput is declared but never applied.
fuzz-seed(Line 35) is not consumed beforeforge test(Line 141), so callers can enable it with no effect.Suggested fix
- name: "Restore the cached build" uses: "sablier-labs/gha-utils/.github/actions/evm-cache@main" with: cache-path: ${{ inputs.cache-path }} fail-on-cache-miss: true + - name: "Generate fuzz seed that changes weekly" + if: ${{ inputs.fuzz-seed }} + run: | + seed=$(($EPOCHSECONDS / 604800)) + echo "FOUNDRY_FUZZ_SEED=$seed" >> "$GITHUB_ENV" + - name: "Run the tests" id: run-tests working-directory: ${{ inputs.working-directory }} run: 'forge test --match-path "${{ inputs.match-path }}"'Also applies to: 138-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-forge-test.yml around lines 35 - 38, The workflow declares an input fuzz-seed but never uses it; update the step that runs the forge test command to conditionally include the fuzz seed flag when inputs.fuzz-seed is set. Locate the job/step that executes "forge test" and modify it to append the appropriate forge flag (e.g., --fuzz-seed <value> or include the flag) based on ${{ inputs.fuzz-seed }} (or a conditional step/args expression) so the declared fuzz-seed input actually affects the forge test invocation..github/workflows/bb-fork-forge-test.yml (4)
85-90:⚠️ Potential issue | 🟠 MajorPin Foundry checkout to an immutable commit SHA.
Line 89 clones a moving branch, which weakens CI reproducibility and supply-chain integrity.
Suggested pinning pattern
- name: Clone Foundry repository working-directory: . run: | if [ ! -d "foundry" ]; then - git clone https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file + git clone https://github.com/BuildBearLabs/foundry.git foundry + git -C foundry checkout --detach <PINNED_COMMIT_SHA> fi shell: bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-fork-forge-test.yml around lines 85 - 90, The workflow step named "Clone Foundry repository" currently clones a moving branch via the git clone command (git clone https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file), which reduces CI reproducibility; change that clone to checkout a specific immutable commit SHA instead: clone the repo (or fetch) and then checkout the provided commit SHA (or replace -b with the exact SHA), optionally using --depth 1 to limit history, and ensure the step verifies the SHA exists (fail fast) so CI uses a fixed, auditable source for the "Clone Foundry repository" step.
142-142:⚠️ Potential issue | 🟠 MajorPin
uses:references to full 40-char SHAs.Line 142 (
@main) and Line 160 (@v1.7.4) are mutable refs. Use immutable commit SHAs for reproducible, hardened workflows.Suggested pinning pattern
- - name: "Restore the cached build" - uses: "sablier-labs/gha-utils/.github/actions/evm-cache@main" + - name: "Restore the cached build" + uses: "sablier-labs/gha-utils/.github/actions/evm-cache@<40-char-commit-sha>" @@ - - name: "Run BB Action CI" - uses: BuildBearLabs/buildbear_x_action@v1.7.4 + - name: "Run BB Action CI" + uses: BuildBearLabs/buildbear_x_action@<40-char-commit-sha>Also applies to: 160-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-fork-forge-test.yml at line 142, Two workflow steps currently reference mutable refs—replace the `uses:` entries that point to `sablier-labs/gha-utils/.github/actions/evm-cache@main` and the other `@v1.7.4` with their corresponding full 40-character commit SHAs; locate the `uses:` lines in the workflow and update each `@<ref>` to `@<40-char-commit-sha>` so the actions are pinned immutably, verifying the SHAs come from the respective repositories/tags before committing.
61-62:⚠️ Potential issue | 🟠 MajorMake
MAINNET_RPC_URLrequired (or conditionally skip fork tests).Line 156 always uses
--fork-url "$MAINNET_RPC_URL", so keeping Line 61-62 optional creates avoidable runtime failures.Suggested fix
secrets: MAINNET_RPC_URL: - required: false + required: trueAlso applies to: 153-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-fork-forge-test.yml around lines 61 - 62, The workflow currently uses --fork-url "$MAINNET_RPC_URL" unconditionally which will fail when MAINNET_RPC_URL is not set; either mark MAINNET_RPC_URL as required in the workflow inputs (change the MAINNET_RPC_URL entry to required: true) or add a conditional that skips the forked-network steps/tests when MAINNET_RPC_URL is empty (update the job/step that invokes --fork-url to run only if env.MAINNET_RPC_URL is defined or branch the step to use the fork flag only when MAINNET_RPC_URL is present); reference the MAINNET_RPC_URL definition and the step that builds the command with --fork-url to implement the fix (also apply the same change where the same --fork-url invocation appears around the other occurrence).
68-77:⚠️ Potential issue | 🟠 MajorScope secrets to step-level instead of job-level.
Lines 69-76 expose secrets to every step, including clone/build steps that don’t need them. Limit secret exposure to only the steps that consume each secret.
Suggested hardening diff
jobs: forge-test: env: - BUILDBEAR_BASE_URL: "${{ secrets.BUILDBEAR_BASE_URL }}" - ETHERSCAN_API_KEY: "${{ secrets.ETHERSCAN_API_KEY }}" FOUNDRY_INVARIANT_DEPTH: ${{ inputs.foundry-invariant-depth }} FOUNDRY_INVARIANT_RUNS: ${{ inputs.foundry-invariant-runs }} FOUNDRY_FUZZ_RUNS: ${{ inputs.foundry-fuzz-runs }} FOUNDRY_PROFILE: ${{ inputs.foundry-profile }} - MAINNET_RPC_URL: "${{ secrets.MAINNET_RPC_URL }}" - ROUTEMESH_API_KEY: "${{ secrets.ROUTEMESH_API_KEY }}" @@ - name: "Run the tests" id: run-tests + env: + MAINNET_RPC_URL: "${{ secrets.MAINNET_RPC_URL }}" + ETHERSCAN_API_KEY: "${{ secrets.ETHERSCAN_API_KEY }}" + ROUTEMESH_API_KEY: "${{ secrets.ROUTEMESH_API_KEY }}" working-directory: ${{ inputs.working-directory }} run: forge test --fork-url "$MAINNET_RPC_URL" --match-path "${{ inputs.match-path }}" continue-on-error: true @@ - name: "Run BB Action CI" uses: BuildBearLabs/buildbear_x_action@v1.7.4 + env: + BUILDBEAR_BASE_URL: "${{ secrets.BUILDBEAR_BASE_URL }}" with: buildbear-api-key: ${{ secrets.BUILDBEAR_API_KEY }} working-directory: ${{ inputs.working-directory }}Also applies to: 153-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bb-fork-forge-test.yml around lines 68 - 77, Job-level env currently exposes sensitive secrets (BUILDBEAR_BASE_URL, ETHERSCAN_API_KEY, MAINNET_RPC_URL, ROUTEMESH_API_KEY, etc.) to every step; move these secret env entries out of the job-level env block and instead add them only to the specific steps that require them (e.g., the verification, deployment, or API-call steps) by adding per-step env entries or using step inputs that reference secrets; apply the same change for the duplicate region around the later job (the block referenced as lines 153-163) so secrets are only scoped to steps that consume BUILDBEAR_BASE_URL, ETHERSCAN_API_KEY, MAINNET_RPC_URL, ROUTEMESH_API_KEY, and any FOUNDRY_* inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/bb-fork-forge-test.yml:
- Around line 55-56: The workflow marks BUILDBEAR_API_KEY as optional but later
unconditionally uses secrets.BUILDBEAR_API_KEY in the step that runs the
Buildbear action (the step referencing secrets.BUILDBEAR_API_KEY around the
block at the later step), so either make BUILDBEAR_API_KEY required: true in the
env/secrets declaration or make the step conditional (e.g., add if:
secrets.BUILDBEAR_API_KEY != '' or equivalent) so the action only runs when the
secret is present; update the BUILDBEAR_API_KEY declaration or the step
condition accordingly to ensure consistency between declaration and usage.
---
Duplicate comments:
In @.github/workflows/bb-forge-test.yml:
- Around line 76-81: The workflow step "Clone Foundry repository" currently
clones a mutable branch (-b feat/better-json-file); change it to pin an
immutable commit SHA by cloning the repo and checking out (or resetting) to the
specific commit SHA instead of relying on the branch name — update the git
commands in that step so they fetch the repo and then run a deterministic
checkout/reset to the identified commit SHA (refer to the "Clone Foundry
repository" step and the git clone invocation when making the change).
- Around line 62-68: The ROUTEMESH_API_KEY is currently set at job-level env
(env: ROUTEMESH_API_KEY) exposing it to all steps; remove ROUTEMESH_API_KEY from
the job-level env block and instead add it to the specific step-level env for
the test step that requires it (the step that runs the Foundry tests), ensuring
only that step references secrets.ROUTEMESH_API_KEY; apply the same change for
the other occurrences called out (the other step env blocks referenced in the
review) so the secret is only present in the minimal step scope.
- Around line 35-38: The workflow declares an input fuzz-seed but never uses it;
update the step that runs the forge test command to conditionally include the
fuzz seed flag when inputs.fuzz-seed is set. Locate the job/step that executes
"forge test" and modify it to append the appropriate forge flag (e.g.,
--fuzz-seed <value> or include the flag) based on ${{ inputs.fuzz-seed }} (or a
conditional step/args expression) so the declared fuzz-seed input actually
affects the forge test invocation.
In @.github/workflows/bb-fork-forge-test.yml:
- Around line 85-90: The workflow step named "Clone Foundry repository"
currently clones a moving branch via the git clone command (git clone
https://github.com/BuildBearLabs/foundry.git -b feat/better-json-file), which
reduces CI reproducibility; change that clone to checkout a specific immutable
commit SHA instead: clone the repo (or fetch) and then checkout the provided
commit SHA (or replace -b with the exact SHA), optionally using --depth 1 to
limit history, and ensure the step verifies the SHA exists (fail fast) so CI
uses a fixed, auditable source for the "Clone Foundry repository" step.
- Line 142: Two workflow steps currently reference mutable refs—replace the
`uses:` entries that point to
`sablier-labs/gha-utils/.github/actions/evm-cache@main` and the other `@v1.7.4`
with their corresponding full 40-character commit SHAs; locate the `uses:` lines
in the workflow and update each `@<ref>` to `@<40-char-commit-sha>` so the
actions are pinned immutably, verifying the SHAs come from the respective
repositories/tags before committing.
- Around line 61-62: The workflow currently uses --fork-url "$MAINNET_RPC_URL"
unconditionally which will fail when MAINNET_RPC_URL is not set; either mark
MAINNET_RPC_URL as required in the workflow inputs (change the MAINNET_RPC_URL
entry to required: true) or add a conditional that skips the forked-network
steps/tests when MAINNET_RPC_URL is empty (update the job/step that invokes
--fork-url to run only if env.MAINNET_RPC_URL is defined or branch the step to
use the fork flag only when MAINNET_RPC_URL is present); reference the
MAINNET_RPC_URL definition and the step that builds the command with --fork-url
to implement the fix (also apply the same change where the same --fork-url
invocation appears around the other occurrence).
- Around line 68-77: Job-level env currently exposes sensitive secrets
(BUILDBEAR_BASE_URL, ETHERSCAN_API_KEY, MAINNET_RPC_URL, ROUTEMESH_API_KEY,
etc.) to every step; move these secret env entries out of the job-level env
block and instead add them only to the specific steps that require them (e.g.,
the verification, deployment, or API-call steps) by adding per-step env entries
or using step inputs that reference secrets; apply the same change for the
duplicate region around the later job (the block referenced as lines 153-163) so
secrets are only scoped to steps that consume BUILDBEAR_BASE_URL,
ETHERSCAN_API_KEY, MAINNET_RPC_URL, ROUTEMESH_API_KEY, and any FOUNDRY_* inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 595da670-afef-4f16-a04d-f74a9434530c
📒 Files selected for processing (4)
.github/workflows/bb-ci-flow.yml.github/workflows/bb-ci-module.yml.github/workflows/bb-forge-test.yml.github/workflows/bb-fork-forge-test.yml
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/bb-ci-flow.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/bb-ci-module.yml
| BUILDBEAR_API_KEY: | ||
| required: false |
There was a problem hiding this comment.
Align BUILDBEAR_API_KEY requirement with unconditional step execution.
BUILDBEAR_API_KEY is optional at Line 55-56, but Line 159-163 always executes the action with that secret. This should be required, or the step should be conditional.
Two valid fixes
secrets:
BUILDBEAR_API_KEY:
- required: false
+ required: true - name: "Run BB Action CI"
+ if: ${{ secrets.BUILDBEAR_API_KEY != '' }}
uses: BuildBearLabs/buildbear_x_action@v1.7.4
with:
buildbear-api-key: ${{ secrets.BUILDBEAR_API_KEY }}
working-directory: ${{ inputs.working-directory }}Also applies to: 159-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/bb-fork-forge-test.yml around lines 55 - 56, The workflow
marks BUILDBEAR_API_KEY as optional but later unconditionally uses
secrets.BUILDBEAR_API_KEY in the step that runs the Buildbear action (the step
referencing secrets.BUILDBEAR_API_KEY around the block at the later step), so
either make BUILDBEAR_API_KEY required: true in the env/secrets declaration or
make the step conditional (e.g., add if: secrets.BUILDBEAR_API_KEY != '' or
equivalent) so the action only runs when the secret is present; update the
BUILDBEAR_API_KEY declaration or the step condition accordingly to ensure
consistency between declaration and usage.
|
As discussed over Telegram with @itsshantanu, Buildbear currently supports debugging integration and unit tests only. While this appears to be a good use case, I would need Buildbear to support debugging following test suites before I can consider merging the PR:
Although Buildbear can still be used for integration and unit tests, debugging these tests is relatively simple since the inputs are almost always fixed, and the test sequence is straightforward. Therefore it offers little to no added value. At the same time, it would still run fork tests in the Buildbear sandbox, using and thereforce consuming our RPC calls. For these reasons, I am closing this PR for now and will wait until Buildbear supports the test suites that are critical for our requirements. |
2 participants
Copy of #1380