fix: pin 35 unpinned action(s),extract 22 unsafe expression(s) to env vars#8606
Open
dagecko wants to merge 1 commit intoAppFlowy-IO:mainfrom
Open
fix: pin 35 unpinned action(s),extract 22 unsafe expression(s) to env vars#8606dagecko wants to merge 1 commit intoAppFlowy-IO:mainfrom
dagecko wants to merge 1 commit intoAppFlowy-IO:mainfrom
Conversation
|
|
Contributor
Reviewer's GuidePins previously unpinned third‑party GitHub Actions to specific commit SHAs and extracts interpolated GitHub/secret expressions out of shell command strings into env vars to harden CI workflows against supply‑chain and expression‑injection attacks, without changing intended behavior. Sequence diagram for secure secret usage in release workflow run stepssequenceDiagram
participant GitHubSecrets
participant Workflow as ReleaseWorkflow
participant JobEnv as JobEnvironment
participant Shell as RunStepShell
participant External as ExternalService
GitHubSecrets->>Workflow: Provide DISCORD, MACOS_CERTIFICATE, MACOS_NOTARY_USER, etc.
Workflow->>JobEnv: Map secrets to env vars (DISCORD, MACOS_CERTIFICATE, REF_NAME, ...)
JobEnv->>Shell: Expose env vars as ${VAR} during run step
Shell->>External: Use env vars in commands (curl, xcrun notarytool, security, etc.)
note over Workflow,Shell: Interpolation inside shell commands changed from ${{ secrets.* }} and ${{ github.ref_name }} to ${VAR} sourced from env
Flow diagram for extracting unsafe expressions to env variablesflowchart TD
Start["Start: Existing workflow run step"] --> Detect["Detect ${{ github.* }} or ${{ secrets.* }} inside run script"]
Detect -->|Found| Extract["Create env variable mapping for each expression"]
Extract --> Replace["Replace occurrences in run script with ${VAR} references"]
Replace --> Result["Result: Shell sees only ${VAR}, GitHub interpolates values into env"]
Detect -->|None| Keep["Keep run step unchanged"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".github/workflows/release.yml" line_range="86-95" />
<code_context>
- dart ./scripts/flutter_release_build/build_flowy.dart exclude-directives . ${{ github.ref_name }}
- cargo make --env APP_VERSION=${{ github.ref_name }} --profile production-windows-x86 appflowy
- dart ./scripts/flutter_release_build/build_flowy.dart include-directives . ${{ github.ref_name }}
+ dart ./scripts/flutter_release_build/build_flowy.dart exclude-directives . ${REF_NAME}
+ cargo make --env APP_VERSION=${REF_NAME} --profile production-windows-x86 appflowy
+ dart ./scripts/flutter_release_build/build_flowy.dart include-directives . ${REF_NAME}
</code_context>
<issue_to_address>
**issue (bug_risk):** Environment variable expansion with `${REF_NAME}` will not work on Windows runners using the default PowerShell shell.
Given the Windows-specific paths and tools, these steps are running under the default PowerShell shell. In PowerShell, environment variables must be referenced as `$Env:REF_NAME`, so `${REF_NAME}` will be passed literally to `dart`, `cargo make`, and `iscc` instead of the tag value.
To fix this, either:
- switch back to GitHub expression interpolation (e.g. `... ${{ github.ref_name }}` / `... ${{ env.REF_NAME }}`), or
- keep `env: REF_NAME: ...` and use PowerShell syntax (`$Env:REF_NAME`), or
- set `shell: bash` on these steps if you want bash-style `${REF_NAME}`.
Otherwise Windows builds will use an incorrect or empty version string.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Author
|
CLA signed.
|
dagecko
force-pushed
the
runner-guard/fix-ci-security
branch
from
c1ae6bf to
48222be
Compare
Author
|
Pushed an update. Saw the feedback on the PowerShell env var expansion and fixed it. The Windows build steps now have We found some additional hardening opportunities post the original PR, so the scope of changes is larger than before. Here's what's in the updated commit:
All changes are mechanical and preserve existing workflow behavior. Happy to walk through any of it if you have questions. I also posted on Twitter about a lot of these hardening techniques if you want to read through the research, and if you like it wouldn't mind a repost. - Chris (dagecko) |
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
2 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.
Security: Harden GitHub Actions workflows
Hey, I found some CI/CD security issues in this repo's GitHub Actions workflows. These are the same vulnerability classes that were exploited in the tj-actions/changed-files supply chain attack. I've been reviewing repos that are affected and submitting fixes where I can.
This PR applies mechanical fixes and flags anything else that needs a manual look. Happy to answer any questions.
Fixes applied
.github/workflows/build_command.yml.github/workflows/commit_lint.yml.github/workflows/docker_ci.yml.github/workflows/flutter_ci.yaml.github/workflows/ios_ci.yaml.github/workflows/mobile_ci.yml.github/workflows/ninja_i18n.yml.github/workflows/release.yml.github/workflows/release.yml.github/workflows/rust_ci.yaml.github/workflows/rust_coverage.yml.github/workflows/translation_notify.ymlAdditional findings (manual review recommended)
| Rule | Severity | File | Description |
| RGS-002 | critical |
.github/workflows/release.yml| Expression Injection via Branch Name or Untrusted Input || RGS-012 | high |
.github/workflows/mobile_ci.yml| Secret Exfiltration via Outbound HTTP Request || RGS-014 | high |
.github/workflows/mobile_ci.yml| Expression Injection via workflow_dispatch Input || RGS-012 | high |
.github/workflows/mobile_ci.yml| Secret Exfiltration via Outbound HTTP Request || RGS-012 | high |
.github/workflows/release.yml| Secret Exfiltration via Outbound HTTP Request || RGS-012 | high |
.github/workflows/release.yml| Secret Exfiltration via Outbound HTTP Request |Why this matters
GitHub Actions workflows that use untrusted input in
run:blocks or reference unpinned third-party actions are vulnerable to code injection and supply chain attacks. These are the same vulnerability classes exploited in the tj-actions/changed-files incident which compromised CI secrets across thousands of repositories.How to verify
Review the diff, each change is mechanical and preserves workflow behavior:
${{ }}expressions fromrun:blocks intoenv:mappings, preventing shell injectionIf this PR is not welcome, just close it and I won't send another.
Summary by Sourcery
Harden GitHub Actions workflows by pinning third-party actions to specific SHAs and isolating dynamic expressions and secrets via environment variables.
CI: