chore(ci): align Yarn 4 workflows and solidity workspace tooling#3999
Open
lionakhnazarov wants to merge 14 commits into
Open
chore(ci): align Yarn 4 workflows and solidity workspace tooling#3999lionakhnazarov wants to merge 14 commits into
lionakhnazarov wants to merge 14 commits into
Conversation
This was referenced May 25, 2026
…ed git setup Updates Solidity CI/doc/npm workflows to use the new setup-git-for-yarn action and aligns ecdsa/random-beacon workspace tooling, Docker, and lockfiles for Yarn 4 compatibility. Co-authored-by: Cursor <cursoragent@cursor.com>
lionakhnazarov
force-pushed
the
stack/testnet4-01-ci-yarn
branch
from
8e4fea1 to
23b83ce
Compare
setup-git-for-yarn previously echoed GIT_CONFIG_GLOBAL=/dev/null (and HOME, XDG_CONFIG_HOME, GIT_CONFIG_NOSYSTEM, GIT_CONFIG_SYSTEM, GIT_CONFIG_COUNT) into $GITHUB_ENV. That propagates to every subsequent step in the job, breaking any downstream "git config --global ..." call: the write lands in /dev/null and the read returns empty. The reusable Solidity docs workflow exercises this path: it calls "git config --global user.email/name" after this action runs, then "git commit -S". Both fail when the global config target is /dev/null. Move the GIT_CONFIG_* sanitisation entirely inside the wrapper script (it already unsets them and exports clean values per invocation, scoped to the git child process). The job env retains only GIT and npm_config_git, which point downstream tooling at the wrapper without poisoning git's config resolution for unrelated steps.
The PR introduced setup-git-for-yarn to centralise git-wrapping for Corepack/Yarn installs, but each workflow still duplicated a ~22-line shell block that exported PATH, scrubbed GIT_CONFIG_*, ran corepack, and invoked yarn install --immutable. That block appeared 13 times across four workflows. Move the install incantation into .github/actions/install-yarn-deps, which takes a working-directory input and also runs setup-git-for-yarn internally so callers do not need a separate step. Each duplicated block collapses to a single `uses:` reference. npm-random-beacon.yml is intentionally left alone in this commit: it runs `yarn upgrade` rather than `yarn install --immutable`, so it does not match the install incantation pattern. Net diff: -351 / +42 across four workflows.
The PR removed setup-node's "cache: yarn" / cache-dependency-path directives across every workflow without a replacement. Yarn 4 stores its content-addressable cache under .yarn/cache (gitignored), so every CI job was re-downloading the full dependency set on every run. Add actions/cache@v4 to the install-yarn-deps composite action, keyed on the workspace's yarn.lock hash. Cache covers .yarn/cache, .yarn/install-state.gz, and node_modules (the workspace uses the node-modules linker). A restore-keys fallback prevents cold-start runs from blocking on a cache miss.
YARN_ENABLE_HARDENED_MODE=0 and YARN_CHECKSUM_BEHAVIOR=ignore turn off Yarn 4's lockfile-integrity verification (poisoned-package detection, checksum mismatch detection). Migrating to Yarn 4 specifically to disable its safety features defeats the supply-chain rationale for the upgrade. The lockfile already carries integrity hashes for the codeload tarballs; ignoring them silences the alarm that would catch real tampering. Drop both env vars from install-yarn-deps and from the previously-dead per-step exports in npm-random-beacon.yml's "Enable Yarn 4" step. If a future "yarn install --immutable" fails with a checksum mismatch, that is a real signal to regenerate the lockfile, not to silence the check.
The reusable Solidity docs workflow used \`hub pull-request\` to open a PR against the destination docs repo. The hub CLI is unmaintained and was removed from ubuntu-22.04 runner images in 2024; it is not present on ubuntu-24.04 images at all. The publish path would fail at runtime the moment GitHub rolls the runner image forward. gh is preinstalled on every hosted runner and is the modern replacement.
The clone step embedded \$GITHUB_TOKEN directly into the HTTPS URL. That token ends up persisted in remote.origin.url inside the cloned repo's .git/config and is visible in any \`git remote -v\` output or process listing during the clone. GitHub Actions redacts known secret strings from job logs, but the token still lands on disk on the runner and in process arguments. Configure gh as a git credential helper for github.com via \`gh auth setup-git\`. gh reads GH_TOKEN from the environment and supplies credentials on demand, so the clone URL stays token-free and the cloned repo's .git/config does not retain credentials.
actions/upload-artifact@master is unpinned: any change pushed to the
action's master branch ships immediately into every CI run, with no
review. Pin to v4 (current major), which also matches GitHub's
recommended supply-chain hygiene for third-party actions.
The two upload-artifact callsites use distinct artifact names
("contracts-after-preprocessing" and "contracts-final-output"), so the
v4 breaking change around per-name uniqueness does not apply.
This file is newly introduced by this PR (previously sourced from keep-network/ci@main). Land it on current major versions of the GitHub-maintained actions rather than the deprecated v3 line. Scope deliberately limited to the new file: pre-existing v3 usage in contracts-*.yml / npm-*.yml is left untouched in this commit to keep the change surgical.
|
Lost in the diff? Review this PR in Change Stack to follow the change map from intent to exact ranges. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds two composite GitHub Actions (git wrapper and unified yarn installer), a reusable Solidity docs workflow, upgrades projects to Yarn 4/Corepack, updates Dockerfiles/manifests, and migrates multiple CI workflows to use the new actions and Yarn commands. ChangesYarn 4.8.1 Upgrade and CI Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (3)
.github/workflows/reusable-solidity-docs.yml (1)
110-148: 💤 Low valueTemplate injection risk in command inputs.
The
preProcessingCommand,postProcessingCommand, andtocOptionsinputs are directly interpolated into shell commands. While this workflow is only callable from within the same repository (reducing risk), if any future caller passes user-controlled data (e.g., from PR titles/body) through these inputs, it could lead to arbitrary code execution.Current callers don't pass these inputs (using defaults), so this is not immediately exploitable. Consider documenting the security expectation in the workflow inputs or using environment variables with proper quoting for
tocOptions:🛡️ Safer approach for tocOptions (example)
- name: Add Table of Contents if: inputs.addTOC == true + env: + TOC_OPTIONS: ${{ inputs.tocOptions }} run: | yarn global add markdown-toc sed -i '2s/^/\ \n/' ./generated-docs/index.md sed -i '2s/^/## Table Of Contents \n/' ./generated-docs/index.md - markdown-toc ${{ inputs.tocOptions }} ./generated-docs/index.md + markdown-toc $TOC_OPTIONS ./generated-docs/index.md🤖 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 @.github/workflows/reusable-solidity-docs.yml around lines 110 - 148, The workflow directly interpolates inputs.preProcessingCommand, inputs.postProcessingCommand and inputs.tocOptions into shell runs (steps named "Execute additional command" and "Add Table of Contents"), creating a template-injection risk; fix by avoiding raw interpolation—validate or restrict those inputs in the workflow inputs schema (reject shell metacharacters), and execute them safely by passing the values via environment variables or as a single quoted positional argument to the shell runner (so the shell does not re-evaluate them), and update the step implementations ("Execute additional command" and "Add Table of Contents") to use the env/quoted argument instead of ${{ inputs.* }} and document the security expectation in the inputs description..github/workflows/npm-random-beacon.yml (1)
34-39: ⚡ Quick winPrefer
./.github/actions/install-yarn-depsover manual Corepack/Yarn bootstrap innpm-random-beacon.yml
npm-random-beacon.ymlonly runssetup-git-for-yarn+corepack enable/yarn --version, then pulls updated packages viayarn upgrade --exact ...without the deterministicyarn install --immutable+ caching/lockfile enforcement provided by./.github/actions/install-yarn-deps(which is used bynpm-ecdsa.yml).♻️ Proposed change
- - uses: ./.github/actions/setup-git-for-yarn - - - name: Enable Yarn 4 (packageManager) - run: | - corepack enable - yarn --version + - uses: ./.github/actions/install-yarn-deps + with: + working-directory: ./solidity/random-beacon🤖 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 @.github/workflows/npm-random-beacon.yml around lines 34 - 39, Replace the manual Corepack/Yarn bootstrapping block that runs corepack enable and yarn --version with the existing reusable action ./ .github/actions/install-yarn-deps: update the workflow to remove the two-run lines and instead add uses: ./.github/actions/install-yarn-deps (same placement as uses: ./.github/actions/setup-git-for-yarn), so the job uses the deterministic install flow (yarn install --immutable plus caching/lockfile enforcement) rather than ad-hoc corepack/yarn commands; ensure any inputs/outputs expected by downstream steps remain intact.solidity/random-beacon/Dockerfile (1)
1-24: Consider running the container as a non-root user.The static analysis tool flags that this Dockerfile does not specify a non-root user. While this is likely a pre-existing pattern and out of scope for the Yarn migration, consider adding a
USERdirective before theENTRYPOINTto follow the principle of least privilege.🤖 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 `@solidity/random-beacon/Dockerfile` around lines 1 - 24, Create and switch to a non-root user before ENTRYPOINT: add a user/group (e.g., "app") after creating WORK_DIR, chown the WORK_DIR and any build artifacts (node_modules, .cache) to that user, and then set USER to that non-root account just prior to the ENTRYPOINT. Ensure the commands around WORK_DIR, the yarn install step, and ENTRYPOINT ("npx", "hardhat") run with the correct ownership/permissions so the non-root user can read/write required files.
🤖 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/reusable-solidity-docs.yml:
- Line 99: The checkout step currently uses "uses: actions/checkout@v4" without
disabling credential persistence; update that step to include
persist-credentials: false so GitHub credentials are not stored in the runner
credential helper and cannot be leaked via artifacts or other uploads—locate the
checkout step referencing uses: actions/checkout@v4 and add the
persist-credentials: false option under that step.
- Around line 162-170: The workflow is using actions/github-script@v6 which is
outdated; update the step referencing "uses: actions/github-script@v6" to v7
(e.g., actions/github-script@v7) and, for stronger supply-chain security,
replace the tag with a pinned commit SHA for the v7 release; keep the existing
github.rest.issues.createComment(...) script unchanged, just update the "uses"
value to the v7 tag or its corresponding commit SHA.
- Around line 215-230: The empty-array check for pr_for_head is wrong: replace
the conditional that compares $pr_for_head to $'[\n\n]' with a proper check for
an empty JSON array (e.g., compare to "[]" or parse with jq), so that when
pr_for_head contains "[]" the script runs the gh pr create branch (the block
invoking gh pr create with --repo, --base, --head, --title, --body) and when it
contains a non-empty array it goes to the else branch; update the conditional
around the pr_for_head variable (the if [[ ... ]] test) accordingly to reliably
detect an empty result.
- Around line 139-143: Replace the deprecated "yarn global add markdown-toc"
invocation (used when inputs.addTOC is true) with a Corepack/Yarn-4-compatible
invocation such as using npx to run markdown-toc (or using pnpm dlx) so TOC
generation works under Yarn 4; update the workflow step that currently runs
"yarn global add markdown-toc" and subsequent "markdown-toc" call to instead
invoke the tool via npx (e.g., "npx markdown-toc ...") so the rest of the
commands (the sed lines and markdown-toc invocation) continue to operate without
relying on yarn global.
---
Nitpick comments:
In @.github/workflows/npm-random-beacon.yml:
- Around line 34-39: Replace the manual Corepack/Yarn bootstrapping block that
runs corepack enable and yarn --version with the existing reusable action ./
.github/actions/install-yarn-deps: update the workflow to remove the two-run
lines and instead add uses: ./.github/actions/install-yarn-deps (same placement
as uses: ./.github/actions/setup-git-for-yarn), so the job uses the
deterministic install flow (yarn install --immutable plus caching/lockfile
enforcement) rather than ad-hoc corepack/yarn commands; ensure any
inputs/outputs expected by downstream steps remain intact.
In @.github/workflows/reusable-solidity-docs.yml:
- Around line 110-148: The workflow directly interpolates
inputs.preProcessingCommand, inputs.postProcessingCommand and inputs.tocOptions
into shell runs (steps named "Execute additional command" and "Add Table of
Contents"), creating a template-injection risk; fix by avoiding raw
interpolation—validate or restrict those inputs in the workflow inputs schema
(reject shell metacharacters), and execute them safely by passing the values via
environment variables or as a single quoted positional argument to the shell
runner (so the shell does not re-evaluate them), and update the step
implementations ("Execute additional command" and "Add Table of Contents") to
use the env/quoted argument instead of ${{ inputs.* }} and document the security
expectation in the inputs description.
In `@solidity/random-beacon/Dockerfile`:
- Around line 1-24: Create and switch to a non-root user before ENTRYPOINT: add
a user/group (e.g., "app") after creating WORK_DIR, chown the WORK_DIR and any
build artifacts (node_modules, .cache) to that user, and then set USER to that
non-root account just prior to the ENTRYPOINT. Ensure the commands around
WORK_DIR, the yarn install step, and ENTRYPOINT ("npx", "hardhat") run with the
correct ownership/permissions so the non-root user can read/write required
files.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7335e747-891e-4704-9b6a-6cab584bb62b
⛔ Files ignored due to path filters (2)
solidity/ecdsa/yarn.lockis excluded by!**/yarn.lock,!**/*.locksolidity/random-beacon/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (24)
.github/actions/install-yarn-deps/action.yml.github/actions/setup-git-for-yarn/action.yml.github/workflows/contracts-ecdsa-docs.yml.github/workflows/contracts-ecdsa.yml.github/workflows/contracts-random-beacon-docs.yml.github/workflows/contracts-random-beacon.yml.github/workflows/npm-ecdsa.yml.github/workflows/npm-random-beacon.yml.github/workflows/reusable-solidity-docs.ymlsolidity/ecdsa/.dockerignoresolidity/ecdsa/.eslintignoresolidity/ecdsa/.gitignoresolidity/ecdsa/.prettierignoresolidity/ecdsa/.yarnrc.ymlsolidity/ecdsa/Dockerfilesolidity/ecdsa/package.jsonsolidity/ecdsa/tsconfig.jsonsolidity/random-beacon/.dockerignoresolidity/random-beacon/.gitignoresolidity/random-beacon/.prettierignoresolidity/random-beacon/.yarnrc.ymlsolidity/random-beacon/Dockerfilesolidity/random-beacon/package.jsonsolidity/random-beacon/tsconfig.json
| run: | ||
| working-directory: .${{ inputs.projectDir }} | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
Add persist-credentials: false to prevent credential leakage.
Without this setting, Git credentials persist in the runner's credential store and can be exfiltrated through artifact uploads.
🛡️ Proposed fix
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4
+ with:
+ persist-credentials: false🧰 Tools
🪛 zizmor (1.25.2)
[warning] 99-99: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 99-99: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/reusable-solidity-docs.yml at line 99, The checkout step
currently uses "uses: actions/checkout@v4" without disabling credential
persistence; update that step to include persist-credentials: false so GitHub
credentials are not stored in the runner credential helper and cannot be leaked
via artifacts or other uploads—locate the checkout step referencing uses:
actions/checkout@v4 and add the persist-credentials: false option under that
step.
Comment on lines
+139
to
+143
| run: | | ||
| yarn global add markdown-toc | ||
| sed -i '2s/^/\ \n/' ./generated-docs/index.md | ||
| sed -i '2s/^/## Table Of Contents \n/' ./generated-docs/index.md | ||
| markdown-toc ${{ inputs.tocOptions }} ./generated-docs/index.md |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is yarn global command available in Yarn 4?
💡 Result:
No, the yarn global command is not available in Yarn 4 [1]. Yarn shifted its focus to project-specific dependency management starting with Yarn 2 (Berry) and continuing through Yarn 4 [1][2]. The global installation of packages was removed because it was deemed outside the scope of the project's goal to support reproducible, project-based environments [1]. To replace the functionality previously provided by yarn global, you should use the following approaches: - For running one-off scripts or CLI tools without installing them, use yarn dlx [1][3]. For example, instead of running yarn global add followed by the command, you can run yarn dlx [1]. - If you need a tool available regularly, the recommended practice is to add it as a project-level dependency in your package.json and execute it using yarn [1][4]. Note: Some external, non-authoritative content may incorrectly suggest that yarn global is still available or applicable to modern versions of Yarn [5], but this is not supported by the official Yarn documentation or CLI reference [1][3].
Citations:
- 1: https://yarnpkg.com/migration/guide
- 2: https://yarnpkg.com/blog/release/4.0
- 3: https://yarnpkg.com/cli
- 4: [Feature] yarn global yarnpkg/berry#821
- 5: https://www.egodaam.com/blog/2026/04/01/how-to-upgrade-all-globally-installed-packages-to-the-latest-version-with-yarn-4c03-29fb5e/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/reusable-solidity-docs.yml"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE" >&2
exit 1
fi
# Show the section around the reported lines and also scan for yarn version/setup
echo "=== Lines 120-170 ==="
nl -ba "$FILE" | sed -n '120,170p'
echo
echo "=== Search for Yarn setup/version in this workflow ==="
rg -n --hidden --no-ignore-vcs -S "yarn(\\s|$)|corepack|setup-node|YARN_|packageManager|node-version|enableCorepack" "$FILE" || true
echo
echo "=== Search for 'yarn set' or corepack usage in repo (limited) ==="
rg -n --hidden --no-ignore-vcs -S "corepack|yarn set version|packageManager: \"yarn|yarn@" .github/workflows || trueRepository: threshold-network/keep-core
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/reusable-solidity-docs.yml"
echo "=== File exists? ==="
test -f "$FILE" && echo "yes: $FILE" || { echo "no: $FILE" >&2; exit 1; }
echo
echo "=== Lines 120-170 (no nl available) ==="
sed -n '120,170p' "$FILE" | awk '{printf "%5d %s\n", NR+119, $0}'
echo
echo "=== Search for Yarn setup/version cues in this workflow ==="
rg -n --hidden --no-ignore-vcs -S "corepack|setup-node|packageManager:|node-version|enableCorepack|YARN_|yarn set|yarn dlx|yarn dlx|yarn global|yarn@" "$FILE" || true
echo
echo "=== Look for packageManager field in repo (limited to package.json) ==="
fd -tf package.json . | head -n 20 | while read -r pj; do
echo "--- $pj"
rg -n '"packageManager"\s*:' "$pj" || true
doneRepository: threshold-network/keep-core
Length of output: 3352
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Check if markdown-toc is already in any package.json dependencies
rg -n --hidden --no-ignore-vcs '"markdown-toc"\s*:' --glob 'package.json' .
# 2) Check for existing usage of markdown-toc CLI in workflows/scripts
rg -n --hidden --no-ignore-vcs 'markdown-toc' .github/workflows .github/actions **/*.{sh,js,ts} **/*.{yml,yaml} 2>/dev/null || true
# 3) Inspect the local action .github/actions/install-yarn-deps (to confirm Yarn version usage)
ACTION_PATH=".github/actions/install-yarn-deps"
if [ -d "$ACTION_PATH" ]; then
echo "=== install-yarn-deps contents ==="
find "$ACTION_PATH" -maxdepth 2 -type f -print
echo
rg -n --hidden --no-ignore-vcs 'yarn|corepack|packageManager|4\.8\.1' "$ACTION_PATH" || true
echo
# show action's action.yml or similar
if [ -f "$ACTION_PATH/action.yml" ]; then
echo "--- action.yml ---"
sed -n '1,200p' "$ACTION_PATH/action.yml"
fi
if [ -f "$ACTION_PATH/action.yaml" ]; then
echo "--- action.yaml ---"
sed -n '1,200p' "$ACTION_PATH/action.yaml"
fi
else
echo "No $ACTION_PATH directory found"
fiRepository: threshold-network/keep-core
Length of output: 53
🏁 Script executed:
#!/bin/bash
set -u
echo "=== 1) Find markdown-toc in any package.json ==="
rg -n --hidden --no-ignore-vcs '"markdown-toc"\s*:' --glob 'package.json' . || true
echo
echo "=== 2) Find markdown-toc CLI usage elsewhere ==="
rg -n --hidden --no-ignore-vcs 'markdown-toc' .github/workflows .github/actions --glob '*.{sh,js,ts}' --glob '*.{yml,yaml}' 2>/dev/null || true
echo
echo "=== 3) Inspect .github/actions/install-yarn-deps to confirm Yarn setup ==="
ACTION_PATH=".github/actions/install-yarn-deps"
if [ -d "$ACTION_PATH" ]; then
find "$ACTION_PATH" -maxdepth 2 -type f -print
echo
rg -n --hidden --no-ignore-vcs 'yarn|corepack|packageManager|4\.8\.1' "$ACTION_PATH" || true
if [ -f "$ACTION_PATH/action.yml" ]; then
echo
echo "--- action.yml ---"
sed -n '1,200p' "$ACTION_PATH/action.yml"
fi
if [ -f "$ACTION_PATH/action.yaml" ]; then
echo
echo "--- action.yaml ---"
sed -n '1,200p' "$ACTION_PATH/action.yaml"
fi
echo
echo "=== 3b) Show first ~200 lines of any shell/js entrypoints in the action ==="
find "$ACTION_PATH" -maxdepth 2 -type f \( -name '*.sh' -o -name '*.js' -o -name '*.ts' \) -print | while read -r f; do
echo
echo "--- $f (head) ---"
sed -n '1,200p' "$f"
done
else
echo "No $ACTION_PATH directory found"
fiRepository: threshold-network/keep-core
Length of output: 3358
Fix TOC generation to work with Yarn 4 (remove yarn global) in .github/workflows/reusable-solidity-docs.yml (lines 139-143).
This step runs with Corepack-managed Yarn 4 (repo packageManager pins yarn@4.8.1+... and .github/actions/install-yarn-deps calls corepack enable), but Yarn 4 removed the yarn global command—so inputs.addTOC == true will fail.
🐛 Proposed fix using npx
- name: Add Table of Contents
if: inputs.addTOC == true
run: |
- yarn global add markdown-toc
sed -i '2s/^/\ \n/' ./generated-docs/index.md
sed -i '2s/^/## Table Of Contents \n/' ./generated-docs/index.md
- markdown-toc ${{ inputs.tocOptions }} ./generated-docs/index.md
+ npx markdown-toc ${{ inputs.tocOptions }} ./generated-docs/index.md📝 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.
Suggested change
| run: | | |
| yarn global add markdown-toc | |
| sed -i '2s/^/\ \n/' ./generated-docs/index.md | |
| sed -i '2s/^/## Table Of Contents \n/' ./generated-docs/index.md | |
| markdown-toc ${{ inputs.tocOptions }} ./generated-docs/index.md | |
| run: | | |
| sed -i '2s/^/\ \n/' ./generated-docs/index.md | |
| sed -i '2s/^/## Table Of Contents \n/' ./generated-docs/index.md | |
| npx markdown-toc ${{ inputs.tocOptions }} ./generated-docs/index.md |
🧰 Tools
🪛 zizmor (1.25.2)
[error] 143-143: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/reusable-solidity-docs.yml around lines 139 - 143, Replace
the deprecated "yarn global add markdown-toc" invocation (used when
inputs.addTOC is true) with a Corepack/Yarn-4-compatible invocation such as
using npx to run markdown-toc (or using pnpm dlx) so TOC generation works under
Yarn 4; update the workflow step that currently runs "yarn global add
markdown-toc" and subsequent "markdown-toc" call to instead invoke the tool via
npx (e.g., "npx markdown-toc ...") so the rest of the commands (the sed lines
and markdown-toc invocation) continue to operate without relying on yarn global.
Comment on lines
+162
to
+170
| uses: actions/github-script@v6 | ||
| with: | ||
| script: | | ||
| github.rest.issues.createComment({ | ||
| issue_number: context.issue.number, | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| body: 'Solidity API documentation preview available in the artifacts of the https://github.com/${{ github.repository}}/actions/runs/${{ github.run_id}} check.' | ||
| }) |
There was a problem hiding this comment.
Update actions/github-script to v7.
Static analysis indicates v6 is outdated. Additionally, consider pinning to a commit SHA for supply chain security.
🔧 Proposed fix
- uses: actions/github-script@v6
+ uses: actions/github-script@v7🧰 Tools
🪛 actionlint (1.7.12)
[error] 162-162: the runner of "actions/github-script@v6" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 zizmor (1.25.2)
[error] 162-162: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/reusable-solidity-docs.yml around lines 162 - 170, The
workflow is using actions/github-script@v6 which is outdated; update the step
referencing "uses: actions/github-script@v6" to v7 (e.g.,
actions/github-script@v7) and, for stronger supply-chain security, replace the
tag with a pinned commit SHA for the v7 release; keep the existing
github.rest.issues.createComment(...) script unchanged, just update the "uses"
value to the v7 tag or its corresponding commit SHA.
Comment on lines
+215
to
+230
| pr_for_head=$(curl -L \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| -H "Authorization: Bearer $GITHUB_TOKEN" \ | ||
| -H "X-GitHub-Api-Version: 2022-11-28" \ | ||
| "https://api.github.com/repos/${{ inputs.destinationRepo }}/pulls?status=open&head=$dest_org:$head_branch") | ||
| if [[ $pr_for_head == $'[\n\n]' ]]; then | ||
| echo "➞ Checked. A PR for the head branch ($head_branch) will be created" | ||
| gh pr create \ | ||
| --repo "${{ inputs.destinationRepo }}" \ | ||
| --base "$base_branch" \ | ||
| --head "$head_branch" \ | ||
| --title "Update Solidity API docs" \ | ||
| --body "Docs updated by workflow: https://github.com/${{ github.repository}}/actions/runs/${{ github.run_id}}" | ||
| else | ||
| echo "➞ Checked. A PR for head branch ($head_branch) already exists and got updated." | ||
| fi |
There was a problem hiding this comment.
Incorrect empty array check will prevent PR creation.
The check $pr_for_head == $'[\n\n]' will never match. GitHub's REST API returns [] for an empty array, not [\n\n]. This means the workflow will always think a PR already exists and skip creation.
🐛 Proposed fix
pr_for_head=$(curl -L \
-H "Accept: application/vnd.github+json" \
-H "Authorization: Bearer $GITHUB_TOKEN" \
-H "X-GitHub-Api-Version: 2022-11-28" \
"https://api.github.com/repos/${{ inputs.destinationRepo }}/pulls?status=open&head=$dest_org:$head_branch")
- if [[ $pr_for_head == $'[\n\n]' ]]; then
+ if [[ $pr_for_head == '[]' ]]; then
echo "➞ Checked. A PR for the head branch ($head_branch) will be created"Alternatively, use jq for more robust parsing:
- if [[ $pr_for_head == $'[\n\n]' ]]; then
+ pr_count=$(echo "$pr_for_head" | jq 'length')
+ if [[ $pr_count -eq 0 ]]; then📝 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.
Suggested change
| pr_for_head=$(curl -L \ | |
| -H "Accept: application/vnd.github+json" \ | |
| -H "Authorization: Bearer $GITHUB_TOKEN" \ | |
| -H "X-GitHub-Api-Version: 2022-11-28" \ | |
| "https://api.github.com/repos/${{ inputs.destinationRepo }}/pulls?status=open&head=$dest_org:$head_branch") | |
| if [[ $pr_for_head == $'[\n\n]' ]]; then | |
| echo "➞ Checked. A PR for the head branch ($head_branch) will be created" | |
| gh pr create \ | |
| --repo "${{ inputs.destinationRepo }}" \ | |
| --base "$base_branch" \ | |
| --head "$head_branch" \ | |
| --title "Update Solidity API docs" \ | |
| --body "Docs updated by workflow: https://github.com/${{ github.repository}}/actions/runs/${{ github.run_id}}" | |
| else | |
| echo "➞ Checked. A PR for head branch ($head_branch) already exists and got updated." | |
| fi | |
| pr_for_head=$(curl -L \ | |
| -H "Accept: application/vnd.github+json" \ | |
| -H "Authorization: Bearer $GITHUB_TOKEN" \ | |
| -H "X-GitHub-Api-Version: 2022-11-28" \ | |
| "https://api.github.com/repos/${{ inputs.destinationRepo }}/pulls?status=open&head=$dest_org:$head_branch") | |
| if [[ $pr_for_head == '[]' ]]; then | |
| echo "➞ Checked. A PR for the head branch ($head_branch) will be created" | |
| gh pr create \ | |
| --repo "${{ inputs.destinationRepo }}" \ | |
| --base "$base_branch" \ | |
| --head "$head_branch" \ | |
| --title "Update Solidity API docs" \ | |
| --body "Docs updated by workflow: https://github.com/${{ github.repository}}/actions/runs/${{ github.run_id}}" | |
| else | |
| echo "➞ Checked. A PR for head branch ($head_branch) already exists and got updated." | |
| fi |
🧰 Tools
🪛 zizmor (1.25.2)
[error] 219-219: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/reusable-solidity-docs.yml around lines 215 - 230, The
empty-array check for pr_for_head is wrong: replace the conditional that
compares $pr_for_head to $'[\n\n]' with a proper check for an empty JSON array
(e.g., compare to "[]" or parse with jq), so that when pr_for_head contains "[]"
the script runs the gh pr create branch (the block invoking gh pr create with
--repo, --base, --head, --title, --body) and when it contains a non-empty array
it goes to the else branch; update the conditional around the pr_for_head
variable (the if [[ ... ]] test) accordingly to reliably detect an empty result.
CI surfaced that Yarn 4 auto-enables hardened mode on public PR
contexts ("workflow is executed from a public pull request"), and
ECDSA's lockfile contains a legitimate npm-descriptor -> git-URL remap
that hardened mode rejects:
YN0078: Invalid resolution ethereumjs-abi@npm:0.6.8 ->
https://github.com/ethereumjs/ethereumjs-abi.git#commit=ee...
The remap exists because ethereumjs-abi@0.6.8 was published broken on
npm; the canonical fix is the git pin, and that is what the lockfile
encodes. Hardened mode considers all npm->git remaps a supply-chain
risk, with no per-entry opt-in.
Restore YARN_ENABLE_HARDENED_MODE=0 with a comment explaining the
reason. Keep YARN_CHECKSUM_BEHAVIOR at its default (the previous
\`=ignore\` was unnecessary; per-package integrity hashes are still
enforced).
Verified locally: \`yarn install --immutable\` succeeds in
solidity/ecdsa with this configuration, fails with YN0078 without it.
piotr-roslaniec
force-pushed
the
stack/testnet4-01-ci-yarn
branch
from
7f74309 to
23b83ce
Compare
2 tasks
Both Solidity workspaces resolved @threshold-network/solidity-contracts
via the floating "development" dist-tag, which has advanced to
1.3.0-dev.15+ and dropped a set of legacy TokenStaking methods that the
test and deployment code still depend on:
- test/utils/operators.ts: TokenStaking.stake, .increaseAuthorization
- test/fixtures/index.ts: TokenStaking.pushNotificationReward,
.setNotificationReward
- deploy/*_approve_*.ts: TokenStaking.approveApplication
Symptoms in CI:
TS2551: Property 'stake' does not exist on type 'TokenStaking'
TS2339: Property 'increaseAuthorization' does not exist on type ...
TS2339: Property 'pushNotificationReward' does not exist on type ...
TS2339: Property 'setNotificationReward' does not exist on type ...
Error: No method named "approveApplication" on contract deployed
as "TokenStaking"
1.3.0-dev.14 is the last published version that still ships all five
methods (verified by inspecting TokenStaking.sol in each published
tarball). Pin both workspaces to that exact version so CI is
deterministic regardless of future dist-tag movement.
Lockfiles regenerated. Verified locally: yarn install --immutable
succeeds, hardhat compile + typechain + test compile pass without the
TS2339/TS2551 errors above.
The docs-publish path in reusable-solidity-docs.yml runs its git operations through the Yarn git wrapper that setup-git-for-yarn installs on $PATH and exports as $GIT/$npm_config_git for the whole job. The wrapper forces GIT_CONFIG_GLOBAL=/dev/null with a throwaway HOME on every git invocation, so the docs-sync step fails: gh auth setup-git and the git config --global user.email/name calls cannot write the global config ("could not lock config file /dev/null"), the credential helper is never registered, and the authenticated git push has no credentials. The earlier token-in-URL approach survived the wrapper because the credentials lived in the URL rather than git config; moving to the credential helper made the path secure but non-functional.
The wrapper is only needed so yarn install can clone git dependencies, which has already happened by the time docs are generated. Add a publish-only step that removes the wrapper binary and repoints $GIT/$npm_config_git at the real git for the remaining publishing steps, restoring the intended credential-helper and tokenless-URL design with no changes to the existing steps.
This path is release-gated (publish == true on a solidity/* release tag) and is not exercised by normal PR or preview runs, so it was verified by static analysis and a local reproduction of the /dev/null config-lock failure rather than an end-to-end release run.
## Summary Follow-up to #3999. Applies the multi-agent / validate-findings review recommendations that were confirmed valid on top of that PR's Yarn 4 migration. Keeps #3999 surgical and lets each fix stand on its own. ## Commits 1. `fix(ci): scope GIT_CONFIG vars to wrapper subshell only` (C1) `setup-git-for-yarn` was exporting `GIT_CONFIG_GLOBAL=/dev/null` to `$GITHUB_ENV`, breaking downstream `git config --global` calls (e.g. docs publish `user.email/name`). Scope GIT_CONFIG vars inside the wrapper subshell only. 2. `refactor(ci): extract Yarn install into composite action` (S1) ~22-line install bash was duplicated 13 times across workflows. New `.github/actions/install-yarn-deps` action takes `working-directory` and centralises the install incantation. Net diff: -351 / +42. 3. `ci: restore Yarn dependency caching via actions/cache` (C4) `cache: "yarn"` was removed from `setup-node` with no replacement, forcing every CI job to re-download deps. Add `actions/cache@v4` keyed on yarn.lock for `.yarn/cache`, `.yarn/install-state.gz`, and `node_modules`. 4. `fix(ci): re-enable Yarn lockfile integrity checks` + `fix(ci): keep HARDENED_MODE disabled, documented` (Sec1) Removed `YARN_CHECKSUM_BEHAVIOR=ignore` (lockfile checksums verified locally as consistent). Kept `YARN_ENABLE_HARDENED_MODE=0` with a documented reason: Yarn 4 auto-enables hardened mode in public PR context, and ECDSA's lockfile contains a legitimate npm-to-git remap for `ethereumjs-abi` (the npm 0.6.8 release was published broken; the git pin is canonical). Hardened mode rejects all such remaps with `YN0078`, regardless of intent. 5. `fix(workflows): replace unmaintained hub with gh pr create` (C6) `hub` was removed from `ubuntu-22.04` runner images in 2024 and is not present on `ubuntu-24.04`. Replace with the preinstalled `gh`. 6. `fix(workflows): use gh credential helper instead of token-in-URL` (Sec3) `git clone https://user:$TOKEN@github.com/...` persisted the token in `remote.origin.url`. Configure `gh auth setup-git` instead; token stays in env, out of `.git/config`. 7. `ci(workflows): pin upload-artifact to v4` (Sec4) `actions/upload-artifact@master` was unpinned. Pin to `@v4`. 8. `ci(workflows): bump checkout and setup-node to v4 in new docs workflow` (C5) Surgical version bump on the newly-introduced `reusable-solidity-docs.yml` only; pre-existing v3 usage elsewhere left alone. ## Notes - Random Beacon `build-and-test` / `deployment-dry-run` and ECDSA `build-and-test` failures with errors like `Property 'pushNotificationReward' does not exist on type 'TokenStaking'` and `No method named "approveApplication"` are **pre-existing** in #3999 (verified against its prior commit's run log). They are not introduced by this PR and are out of scope here. - `setup-node` is left on v3 in `contracts-*.yml` and `npm-*.yml` to match repo convention. Bumping there can be a follow-up. ## Test plan - [ ] CI install/lint/slither/docs jobs green - [ ] Sec3 / Sec4 / C5 / C6 only affect docs workflow (not exercised on normal PRs unless `publish == true`); verify by inspection
Yarn 4 (Berry) renamed the classic 'yarn upgrade' command to 'yarn up' and removed the old name, so 'yarn upgrade ...' exits with a usage error under the Corepack-pinned yarn@4.8.1 this stack enables. Seven calls survived the Yarn-4 migration, each on a job that runs install-yarn-deps (corepack-enabling Yarn 4) before the upgrade step: - npm-random-beacon.yml: npm publish on push to main - contracts-ecdsa.yml / contracts-random-beacon.yml: workflow_dispatch deployment-testnet jobs None of these paths run on pull_request, so PR CI stays green and the breakage would only surface post-merge on publish or manual deploys. Verified under yarn@4.8.1 that all argument forms work with 'yarn up': plain (--exact), version-pinned multi-package, and git-ref descriptors (@github:owner/repo#ref). Out of scope (same bug class, follow-up): 'yarn global add' in reusable-solidity-docs.yml is also Yarn-4-removed but is gated behind addTOC and dormant for all current callers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack Context
This is PR 1/4 in the reorganization stack.
Base:
mainNext: #4000
What Changed
1) CI + workflow execution model
.github/actions/setup-git-for-yarn/action.yml..github/workflows/reusable-solidity-docs.yml..github/workflows/contracts-ecdsa.yml.github/workflows/contracts-random-beacon.yml.github/workflows/contracts-ecdsa-docs.yml.github/workflows/contracts-random-beacon-docs.yml.github/workflows/npm-ecdsa.yml.github/workflows/npm-random-beacon.yml2) Solidity workspace tooling alignment
solidity/ecdsa/.yarnrc.ymlsolidity/random-beacon/.yarnrc.ymlsolidity/ecdsa/.dockerignore,.eslintignore,.gitignore,.prettierignore,Dockerfilesolidity/random-beacon/.dockerignore,.gitignore,.prettierignore,Dockerfile3) Dependency metadata and lockfiles
solidity/ecdsa/package.json,solidity/ecdsa/tsconfig.jsonsolidity/random-beacon/package.json,solidity/random-beacon/tsconfig.jsonsolidity/ecdsa/yarn.locksolidity/random-beacon/yarn.lockOut of Scope
Test Plan
Summary by CodeRabbit
New Features
Chores
Other