Harden CI workflow permissions, command injection vectors, and schema validation in chains - #1
Open
mozluk wants to merge 2 commits into
Open
Harden CI workflow permissions, command injection vectors, and schema validation in chains#1mozluk wants to merge 2 commits into
chains#1mozluk wants to merge 2 commits into
Conversation
… validation in `chains`
### Description
This pull request addresses Medium and Low severity security and reliability findings from the workspace audit targeting the `chains` repository[cite: 28]. Previously, GitHub Actions workflows ran with broad default token scopes, shell scripts interpolated untrusted step outputs directly into bash text (creating command injection vectors), caching keys referenced non-existent paths, and the JSON schema validator aborted on the first encountered error with truncated negative exit codes[cite: 28]. This PR introduces least-privilege token permissions across all workflows, isolates shell inputs, pins tooling versions, and optimizes schema checking[cite: 28].
### Key Changes & Remediations
#### 1. Actions Injection Defense & Build Optimization (`.github/workflows/build.yml`)
* **Environment Variable Isolation (F19):** Replaced direct interpolation of `${{ steps.changed-files.outputs.all_changed_files }}` in the shell run block with an `env: ALL_CHANGED_FILES` mapping[cite: 28, 34]. Added `set -f` to prevent glob/pathname expansion over filenames with wildcards (`*`, `?`)[cite: 28, 34].
* **Loop Hoisting:** Moved `./gradlew clean` outside the per-file verification loop, resolving an issue where every iteration discarded previous artifacts and triggered cold rebuilds for each changed chain file[cite: 28, 34].
#### 2. Least-Privilege Workflow Permissions (F20)
* **Explicit Scoping (`*.yml`):** Added explicit `permissions` blocks across all workflows to restrict the default broad token access[cite: 28]:
* `build.yml`, `prettier_check.yml`, `validate_json.yml`: restricted strictly to `contents: read`[cite: 34, 39, 41].
* `pr_intro_comment.yml`, `post_merge_comment.yml`: narrowed to `pull-requests: write`[cite: 37, 38].
* `stale.yml`: narrowed to `issues: write` and `pull-requests: write`[cite: 40].
* `deploy.yml`: limited strictly to `contents: write` for GitHub Pages publication[cite: 36].
#### 3. Reproducible CI Caching & Dependency Pinning (F24)
* **Cache Key Integrity (`prettier_check.yml`):** Corrected the npm cache key from the non-existent `**/workflows/prettier.yml` to `.github/workflows/prettier_check.yml`[cite: 28, 39]. Pinned the execution to `npx --yes prettier@3` to prevent unpinned major version shifts from failing checks on older files[cite: 28, 39].
* **Deterministic Installation (`validate_json.yml`):** Keyed npm cache directly to `tools/package-lock.json` and replaced `npm install` with `npm ci` to guarantee reproducible builds matching committed lockfiles[cite: 28, 41].
#### 4. Defensive Schema Checking & Performance (`tools/schemaCheck.js`)
* **Batch Diagnostics (F23):** Replaced early exits with an aggregated error collection loop, reporting all malformed files and schema mismatches in a single CI run instead of failing on the first error[cite: 28, 42].
* **Defensive JSON Parsing:** Wrapped `JSON.parse` in a `try/catch` block to identify and report exact corrupt filenames rather than aborting with anonymous parser exceptions[cite: 28, 42]. Filtered directory listings to `.json` extensions to ignore OS artifacts (`.DS_Store`, `.swp`)[cite: 42].
* **CAIP-2 Identifiers & Coercion:** Explicitly validated file names against CAIP-2 structure (`<namespace>-<reference>.json`) and compared chain IDs numerically to avoid loose type coercion[cite: 42].
* **Compiler Hoisting & Clean Exit:** Hoisted `ajv.compile(schema)` outside the iteration loop (saving 2,600+ schema compilations) and replaced `exit(-1)` with `exit(1)` to avoid OS status code truncation to 255[cite: 28, 42].
### How to Review
1. **CI Permissions & Shell Execution:** Inspect `.github/workflows/build.yml` and verify that `ALL_CHANGED_FILES` is passed via `env:` with `set -f` enabled, and verify `permissions:` blocks across all workflow files[cite: 34, 37, 38, 39, 40, 41].
2. **Cache Keys:** Verify that cache hashing in `prettier_check.yml` and `validate_json.yml` points to existing repository files (`prettier_check.yml` and `tools/package-lock.json`)[cite: 39, 41].
3. **Schema Validator:** Check `tools/schemaCheck.js` to ensure errors accumulate gracefully without early exits and that `exit(1)` is emitted on failure[cite: 42].
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
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.
Description
This pull request addresses Medium and Low severity security and reliability findings from the workspace audit targeting the
chainsrepository[cite: 28]. Previously, GitHub Actions workflows ran with broad default token scopes, shell scripts interpolated untrusted step outputs directly into bash text (creating command injection vectors), caching keys referenced non-existent paths, and the JSON schema validator aborted on the first encountered error with truncated negative exit codes[cite: 28]. This PR introduces least-privilege token permissions across all workflows, isolates shell inputs, pins tooling versions, and optimizes schema checking[cite: 28].Key Changes & Remediations
1. Actions Injection Defense & Build Optimization (
.github/workflows/build.yml)${{ steps.changed-files.outputs.all_changed_files }}in the shell run block with anenv: ALL_CHANGED_FILESmapping[cite: 28, 34]. Addedset -fto prevent glob/pathname expansion over filenames with wildcards (*,?)[cite: 28, 34]../gradlew cleanoutside the per-file verification loop, resolving an issue where every iteration discarded previous artifacts and triggered cold rebuilds for each changed chain file[cite: 28, 34].2. Least-Privilege Workflow Permissions (F20)
*.yml): Added explicitpermissionsblocks across all workflows to restrict the default broad token access[cite: 28]:build.yml,prettier_check.yml,validate_json.yml: restricted strictly tocontents: read[cite: 34, 39, 41].pr_intro_comment.yml,post_merge_comment.yml: narrowed topull-requests: write[cite: 37, 38].stale.yml: narrowed toissues: writeandpull-requests: write[cite: 40].deploy.yml: limited strictly tocontents: writefor GitHub Pages publication[cite: 36].3. Reproducible CI Caching & Dependency Pinning (F24)
prettier_check.yml): Corrected the npm cache key from the non-existent**/workflows/prettier.ymlto.github/workflows/prettier_check.yml[cite: 28, 39]. Pinned the execution tonpx --yes prettier@3to prevent unpinned major version shifts from failing checks on older files[cite: 28, 39].validate_json.yml): Keyed npm cache directly totools/package-lock.jsonand replacednpm installwithnpm cito guarantee reproducible builds matching committed lockfiles[cite: 28, 41].4. Defensive Schema Checking & Performance (
tools/schemaCheck.js)JSON.parsein atry/catchblock to identify and report exact corrupt filenames rather than aborting with anonymous parser exceptions[cite: 28, 42]. Filtered directory listings to.jsonextensions to ignore OS artifacts (.DS_Store,.swp)[cite: 42].<namespace>-<reference>.json) and compared chain IDs numerically to avoid loose type coercion[cite: 42].ajv.compile(schema)outside the iteration loop (saving 2,600+ schema compilations) and replacedexit(-1)withexit(1)to avoid OS status code truncation to 255[cite: 28, 42].How to Review
.github/workflows/build.ymland verify thatALL_CHANGED_FILESis passed viaenv:withset -fenabled, and verifypermissions:blocks across all workflow files[cite: 34, 37, 38, 39, 40, 41].prettier_check.ymlandvalidate_json.ymlpoints to existing repository files (prettier_check.ymlandtools/package-lock.json)[cite: 39, 41].tools/schemaCheck.jsto ensure errors accumulate gracefully without early exits and thatexit(1)is emitted on failure[cite: 42].