From 9034c5b8bcbb6040e718daab9a1da615244d22b9 Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Fri, 2 Jan 2026 16:07:16 -0500 Subject: [PATCH 01/11] dig helper function for attributes/keys with special characters --- lib/claws/base_rule.rb | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/lib/claws/base_rule.rb b/lib/claws/base_rule.rb index 4706a57..bff2959 100644 --- a/lib/claws/base_rule.rb +++ b/lib/claws/base_rule.rb @@ -1,10 +1,10 @@ class BaseRule attr_accessor :on_workflow, :on_job, :on_step, :configuration - def self.parse_rule(rule) # rubocop:disable Metrics/AbcSize + def self.parse_rule(rule) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity ExpressionParser.parse_expression(rule).tap do |expression| expression.instance_eval do - def ctx # rubocop:disable Metrics/AbcSize + def ctx # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity @ctx ||= Context.new( default: {}, methods: { @@ -15,7 +15,26 @@ def ctx # rubocop:disable Metrics/AbcSize difference: ->(arr1, arr2) { arr1.difference arr2 }, intersection: ->(arr1, arr2) { arr1.intersection arr2 }, get_key: ->(arr, key) { (arr || {}).fetch(key, nil) }, - count: ->(n) { n.length } + count: ->(n) { n.length }, + dig: lambda { |object, path, default: nil| + # sometimes we might want to traverse the object as if it were a hash + # sometimes we might want to traverse it as a Ruby object + # annoying up front, but the edge cases are few and keeps expressions simple + path.to_s.split(".").reduce(object) do |current, part| + return nil if current.nil? + + if current.is_a?(Hash) + # Prefer exact string key, then symbol key + if current.key?(part) + current[part] + elsif current.key?(part.to_sym) + current[part.to_sym] + end + else + current.respond_to?(part) ? current.public_send(part) : default + end + end + } } ) end From 1de0f66a96b8633ad07ea828e634d8d99084b74a Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Fri, 2 Jan 2026 16:32:15 -0500 Subject: [PATCH 02/11] implicit persist credentials --- lib/claws/rule.rb | 1 + .../rule/implicit_persist_credentials.rb | 27 +++++++++++ .../rule/implicit_persist_credentials_spec.rb | 47 +++++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 lib/claws/rule/implicit_persist_credentials.rb create mode 100644 spec/claws/rule/implicit_persist_credentials_spec.rb diff --git a/lib/claws/rule.rb b/lib/claws/rule.rb index fcd0ba5..9823ae9 100644 --- a/lib/claws/rule.rb +++ b/lib/claws/rule.rb @@ -12,3 +12,4 @@ require "claws/rule/bulk_permissions" require "claws/rule/shellcheck" require "claws/rule/checkout_with_static_credentials" +require "claws/rule/implicit_persist_credentials" diff --git a/lib/claws/rule/implicit_persist_credentials.rb b/lib/claws/rule/implicit_persist_credentials.rb new file mode 100644 index 0000000..c85735c --- /dev/null +++ b/lib/claws/rule/implicit_persist_credentials.rb @@ -0,0 +1,27 @@ +module Claws + module Rule + class ImplicitPersistCredentials < BaseRule + description <<~DESC + By default, actions/checkout will store generated credentials to disk so that + subsequent git operations will not require reauthentication. These credentials + will be available to subsequent steps and jobs. This may be undesirable and + potentially unsafe in scenarios where these credentials may be accessible to + untrusted code. In these cases, if these credentials are stolen they can be used + externally by an attacker to clone repositories that would otherwise have been + inaccessible. + + If you know you will not need to access this repository for the rest of your + workflow, consider setting `persist-credentials` to false. Conversely, + explicitly set it to true if you know you will need these credentials. + + For more information: + https://github.com/betterment/claws/blob/main/README.md#implicitpersistcredentials + DESC + + on_step %( + $step.meta.action.name == "actions/checkout" && + !contains([true, false], dig($step, "with.persist-credentials")) + ), highlight: "uses" + end + end +end diff --git a/spec/claws/rule/implicit_persist_credentials_spec.rb b/spec/claws/rule/implicit_persist_credentials_spec.rb new file mode 100644 index 0000000..345d586 --- /dev/null +++ b/spec/claws/rule/implicit_persist_credentials_spec.rb @@ -0,0 +1,47 @@ +RSpec.describe Claws::Rule::ImplicitPersistCredentials do + before do + load_detection + end + + context "with default configuration" do + it "flags a checkout with no explicit setting for persist-credentials" do + violations = analyze(<<~YAML) + name: Check out the current repository + + on: [pull_request] + + jobs: + build: + steps: + - uses: actions/checkout@v6 + - run: | + rake setup + rake spec + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(8) + expect(violations[0].name).to eq("ImplicitPersistCredentials") + end + + it "doesn't flag a checkout if persist-credentials is set to false" do + violations = analyze(<<~YAML) + name: Check out the current repository + + on: [pull_request] + + jobs: + build: + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + - run: | + rake setup + rake spec + YAML + + expect(violations.count).to eq(0) + end + end +end From 266636a8221f8d2e3c4009c216c2dabcdda59af1 Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Fri, 2 Jan 2026 16:53:56 -0500 Subject: [PATCH 03/11] global permissions block rule --- lib/claws/rule.rb | 1 + lib/claws/rule/global_permissions_block.rb | 34 ++++++ .../rule/global_permissions_block_spec.rb | 104 ++++++++++++++++++ 3 files changed, 139 insertions(+) create mode 100644 lib/claws/rule/global_permissions_block.rb create mode 100644 spec/claws/rule/global_permissions_block_spec.rb diff --git a/lib/claws/rule.rb b/lib/claws/rule.rb index 9823ae9..efce34c 100644 --- a/lib/claws/rule.rb +++ b/lib/claws/rule.rb @@ -13,3 +13,4 @@ require "claws/rule/shellcheck" require "claws/rule/checkout_with_static_credentials" require "claws/rule/implicit_persist_credentials" +require "claws/rule/global_permissions_block" diff --git a/lib/claws/rule/global_permissions_block.rb b/lib/claws/rule/global_permissions_block.rb new file mode 100644 index 0000000..3cef6ac --- /dev/null +++ b/lib/claws/rule/global_permissions_block.rb @@ -0,0 +1,34 @@ +module Claws + module Rule + class GlobalPermissionsBlock < BaseRule + description <<~DESC + Permissions should be set at the job level, not globally at the workflow level. + Because jobs will often need varying permissions, it's better to specify a set + of permissions for each individual job, minimizing potential misuse from + untrusted code in a job with permissions it never needed in the first place. + + This rule will flag workflows that have multiple jobs and a root level + permissions block. If there is a root level permissions block but just one job, + it will not be flagged. + + For more information: + https://github.com/betterment/claws/blob/main/README.md#globalpermissionsblock + DESC + + on_workflow :test_root_level_permissions + + def test_root_level_permissions(workflow:, job:, step:) # rubocop:disable Lint/UnusedMethodArgument + root_permission_block_line = workflow.keys.filter { |x| x == "permissions" }.first&.line + return if root_permission_block_line.nil? + + job_count = workflow["jobs"]&.count || 0 + return if job_count < 2 + + Violation.new( + line: root_permission_block_line, + description: + ) + end + end + end +end diff --git a/spec/claws/rule/global_permissions_block_spec.rb b/spec/claws/rule/global_permissions_block_spec.rb new file mode 100644 index 0000000..110dec3 --- /dev/null +++ b/spec/claws/rule/global_permissions_block_spec.rb @@ -0,0 +1,104 @@ +RSpec.describe Claws::Rule::GlobalPermissionsBlock do + before do + load_detection + end + + context "with default configuration" do + it "flags a workflow with a top level permissions block if there is more than one job" do + violations = analyze(<<~YAML) + name: publish docs + + on: + push: + branches: + - main + + permissions: + contents: read + pages: write + id-token: write + + jobs: + build: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + + deploy: + needs: build + runs-on: ubuntu-latest + environment: + name: github-pages + steps: + - name: Deploy to GitHub Pages + id: deployment + uses: actions/deploy-pages@v4 + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(8) + expect(violations[0].name).to eq("GlobalPermissionsBlock") + end + + it "does not flag a workflow with a top level permissions block if there is just one job" do + violations = analyze(<<~YAML) + name: pretend to publish docs + + on: + push: + branches: + - main + + permissions: + contents: read + pages: write + id-token: write + + jobs: + build: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + YAML + + expect(violations.count).to eq(0) + end + + it "does not flag a workflow if there is no top level permissions block" do + violations = analyze(<<~YAML) + name: publish docs + + on: + push: + branches: + - main + + jobs: + permissions: + contents: read + pages: write + id-token: write + + build: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + + deploy: + needs: build + runs-on: ubuntu-latest + environment: + name: github-pages + steps: + - name: Deploy to GitHub Pages + id: deployment + uses: actions/deploy-pages@v4 + YAML + + expect(violations.count).to eq(0) + end + end +end From 3ddc50337b54dfab028ebb31f2d93148a6fd2282 Mon Sep 17 00:00:00 2001 From: "Omar A." <47008591+6f6d6172@users.noreply.github.com> Date: Fri, 2 Jan 2026 17:55:30 -0500 Subject: [PATCH 04/11] ImplicitPersistCredentials and GlobalPermissionsBlock docs --- README.md | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/README.md b/README.md index d4f681e..d883555 100644 --- a/README.md +++ b/README.md @@ -395,6 +395,121 @@ This rule only looks for user supplied branches being checked out for `pull_requ |-------------|------------------------|---------------------------------------------------------------| | `risky_events` | ["pull_request_target", "workflow_dispatch"] | An array of Github events you consider risky. | +### ImplicitPersistCredentials + +By default, all versions of the official [`actions/checkout`](https://github.com/actions/checkout) have a default value of `true` for the `persist-credentials` setting. This means the temporary credentials generated to clone a repository's source code will be written to disk. This is necessary if you intend on using the `git` command line tool to further interact with this repository (e.g. check out a specific branch or push new commits to it). However, if you don't intend on doing this, this opens up an unnecessary risk. Untrusted or otherwise malicious code can find these temporary credentials and use them to access source code and other resources that would otherwise not have been exposed by your workflow. For example, if during a supply chain attack someone steals your job's environment variables, they may be able to use these credentials from their own system to access private repositories. + +For example, take the following workflow: + +```yaml +name: Ruby CI (PR) + +on: [pull_request] + +jobs: + bundle-install: + name: Bundle install + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + + - name: Install dependencies + run: bundle install +``` + +Because `persist-credentials` is not specified, its value is `true` by default. That means the temporary Github credentials used to clone the repository will be either written to `.git/config` or to a temporary directory. Both of these locations will be accessible to subsequent steps, so if for example we were installing a malicious dependency with the `bundle install` command, it could find those credentials on disk and send them to someone via the network, letting them clone this repository and potentially access even other repositories. In this workflow, we aren't interacting with git beyond that checkout, so we don't need to persist the credential. We can fix this by setting `persist-credentials` to `false`: + +```yaml +# ... + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + persist-credentials: false +``` + +However, if you do need to interact with the repository via git, you should explicitly set `persist-credentials` to `true`. This way, you signal to your reviewers that your workflow interacts with the local checkout in a way that needs this credential. + +Note, there is [a Github Issue tracking this](https://github.com/actions/checkout/issues/485) that has been open for a few years by now. + +### GlobalPermissionsBlock + +[The permissions block](https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#permissions) dictates which permissions a workflow or a job have or don't have. When permissions are defined at the workflow level, each job gets the same set of permissions, potentially giving more access than a job or a step truly needs. Instead, permissions should be defined at the job level, making sure each job and step has access only to the permissions it needs. This can minimize the impact of untrusted code using an overly permissive `$GITHUB_TOKEN`, such as during a supply chain attack. + +For example, take the following workflow: + +```yaml +name: Build and Deploy + +on: [push] + +permissions: + id-token: write + +jobs: + build: + name: Build + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Install dependencies + run: bundle install + + - name: Upload build artifact to GitHub + uses: actions/upload-artifact@v4 + with: + name: ${{ env.ARTIFACT_NAME }}-${{ steps.meta.outputs.version }} + path: ${{ env.ARTIFACT_NAME }}.tgz + + deploy: + name: Deploy + needs: build + + steps: + - name: Configure AWS credentials (OIDC) + uses: aws-actions/configure-aws-credentials@v4 + with: + role-to-assume: ${{ env.ROLE_TO_ASSUME }} + aws-region: ${{ env.AWS_REGION }} + + - name: Upload release artifact to S3 + run: | + aws s3 cp "${ARTIFACT_NAME}.tgz" "s3://${S3_BUCKET}/${{ needs.build.outputs.s3_key }}" +``` + +This sample workflow deploys to AWS using OIDC, so it needs the `id-token: write` permission to generate temporary AWS credentials. However, because this permission is defined at the workflow level, the `build` job can also use OIDC to grab temporary AWS credentials. This means if this job has some kind of vulnerability, an attacker could use it to gain access to an AWS environment that would otherwise have been entirely inaccessible. + +To remedy this, we should move the `permissions:` block into the specific job that actually needs this permission: + +```yaml +# ... + deploy: + name: Deploy + needs: build + permissions: + id-token: write + + steps: + - name: Configure AWS credentials (OIDC) + uses: aws-actions/configure-aws-credentials@v4 + with: + role-to-assume: ${{ env.ROLE_TO_ASSUME }} + aws-region: ${{ env.AWS_REGION }} + + - name: Upload release artifact to S3 + run: | + aws s3 cp "${ARTIFACT_NAME}.tgz" "s3://${S3_BUCKET}/${{ needs.build.outputs.s3_key }}" +``` + +This limits AWS OIDC access to just `deploy`. + ## Walkthrough Let's start with a minimal configuration file that enables some basic Rules. From c95bc6beb7d1a46390a0bc80397a8aa11acf248a Mon Sep 17 00:00:00 2001 From: "Omar A." <47008591+6f6d6172@users.noreply.github.com> Date: Fri, 2 Jan 2026 18:08:49 -0500 Subject: [PATCH 05/11] workflow syntax fix Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- spec/claws/rule/global_permissions_block_spec.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/claws/rule/global_permissions_block_spec.rb b/spec/claws/rule/global_permissions_block_spec.rb index 110dec3..3b391ef 100644 --- a/spec/claws/rule/global_permissions_block_spec.rb +++ b/spec/claws/rule/global_permissions_block_spec.rb @@ -76,12 +76,11 @@ - main jobs: - permissions: - contents: read - pages: write - id-token: write - build: + permissions: + contents: read + pages: write + id-token: write runs-on: ubuntu-latest steps: - name: Checkout From 669792387779fb0c441cfdac2751960bb7520eca Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Fri, 2 Jan 2026 18:13:38 -0500 Subject: [PATCH 06/11] return default when working with hashes too --- lib/claws/base_rule.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/claws/base_rule.rb b/lib/claws/base_rule.rb index bff2959..a43311d 100644 --- a/lib/claws/base_rule.rb +++ b/lib/claws/base_rule.rb @@ -21,7 +21,7 @@ def ctx # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics # sometimes we might want to traverse it as a Ruby object # annoying up front, but the edge cases are few and keeps expressions simple path.to_s.split(".").reduce(object) do |current, part| - return nil if current.nil? + return default if current.nil? if current.is_a?(Hash) # Prefer exact string key, then symbol key @@ -29,6 +29,8 @@ def ctx # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics current[part] elsif current.key?(part.to_sym) current[part.to_sym] + else + default end else current.respond_to?(part) ? current.public_send(part) : default From c963421d01b05f65034a74c3da949a994d4afa85 Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 5 Jan 2026 13:14:22 -0500 Subject: [PATCH 07/11] v4 -> v6 just to appease the robot --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index d883555..a45aeaf 100644 --- a/README.md +++ b/README.md @@ -413,7 +413,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@v6 - name: Set up Ruby uses: ruby/setup-ruby@v1 @@ -428,7 +428,7 @@ Because `persist-credentials` is not specified, its value is `true` by default. # ... steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@v6 with: persist-credentials: false ``` @@ -457,7 +457,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v6 - name: Install dependencies run: bundle install From 473e0da7ae67e7b9223355d48c5d3d94fc3bf0fd Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 5 Jan 2026 13:14:54 -0500 Subject: [PATCH 08/11] test case for default global permissions --- .../rule/global_permissions_block_spec.rb | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/spec/claws/rule/global_permissions_block_spec.rb b/spec/claws/rule/global_permissions_block_spec.rb index 3b391ef..d605e16 100644 --- a/spec/claws/rule/global_permissions_block_spec.rb +++ b/spec/claws/rule/global_permissions_block_spec.rb @@ -41,6 +41,48 @@ expect(violations[0].name).to eq("GlobalPermissionsBlock") end + it "flags a workflow for global permissions even if some jobs specify their own" do + violations = analyze(<<~YAML) + name: publish docs + + on: + push: + branches: + - main + + # default for all jobs + permissions: + contents: read + pages: write + + jobs: + build: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + + deploy: + needs: build + runs-on: ubuntu-latest + # override defaults + permissions: + contents: read + pages: write + id-token: write + environment: + name: github-pages + steps: + - name: Deploy to GitHub Pages + id: deployment + uses: actions/deploy-pages@v4 + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(9) + expect(violations[0].name).to eq("GlobalPermissionsBlock") + end + it "does not flag a workflow with a top level permissions block if there is just one job" do violations = analyze(<<~YAML) name: pretend to publish docs From a95da7a00f09870514a97936a0c567f6c421fbdb Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 5 Jan 2026 13:15:22 -0500 Subject: [PATCH 09/11] additional test case for different values of persist-credentials --- .../rule/implicit_persist_credentials_spec.rb | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/spec/claws/rule/implicit_persist_credentials_spec.rb b/spec/claws/rule/implicit_persist_credentials_spec.rb index 345d586..c58c8a2 100644 --- a/spec/claws/rule/implicit_persist_credentials_spec.rb +++ b/spec/claws/rule/implicit_persist_credentials_spec.rb @@ -24,6 +24,26 @@ expect(violations[0].name).to eq("ImplicitPersistCredentials") end + it "doesn't flag a checkout if persist-credentials is set to true" do + violations = analyze(<<~YAML) + name: Check out the current repository + + on: [pull_request] + + jobs: + build: + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + - run: | + rake setup + rake spec + YAML + + expect(violations.count).to eq(0) + end + it "doesn't flag a checkout if persist-credentials is set to false" do violations = analyze(<<~YAML) name: Check out the current repository @@ -43,5 +63,27 @@ expect(violations.count).to eq(0) end + + it "flags a checkout if persist-credentials is set to a non-boolean" do + violations = analyze(<<~YAML) + name: Check out the current repository + + on: [pull_request] + + jobs: + build: + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: hello + - run: | + rake setup + rake spec + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(8) + expect(violations[0].name).to eq("ImplicitPersistCredentials") + end end end From fbd1cfdcd1f2367554bdbc12f236593576b39821 Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 5 Jan 2026 13:36:37 -0500 Subject: [PATCH 10/11] alright you got me there --- spec/claws/rule/implicit_persist_credentials_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/claws/rule/implicit_persist_credentials_spec.rb b/spec/claws/rule/implicit_persist_credentials_spec.rb index c58c8a2..461cc46 100644 --- a/spec/claws/rule/implicit_persist_credentials_spec.rb +++ b/spec/claws/rule/implicit_persist_credentials_spec.rb @@ -35,7 +35,7 @@ steps: - uses: actions/checkout@v6 with: - persist-credentials: false + persist-credentials: true - run: | rake setup rake spec From d5c0d7c8ae0285e969bcf2530c6d258d8143d33c Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 5 Jan 2026 13:40:52 -0500 Subject: [PATCH 11/11] forgot how kw args work for a sec --- lib/claws/base_rule.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/claws/base_rule.rb b/lib/claws/base_rule.rb index a43311d..32bc236 100644 --- a/lib/claws/base_rule.rb +++ b/lib/claws/base_rule.rb @@ -16,7 +16,7 @@ def ctx # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics intersection: ->(arr1, arr2) { arr1.intersection arr2 }, get_key: ->(arr, key) { (arr || {}).fetch(key, nil) }, count: ->(n) { n.length }, - dig: lambda { |object, path, default: nil| + dig: lambda { |object, path, default = nil| # sometimes we might want to traverse the object as if it were a hash # sometimes we might want to traverse it as a Ruby object # annoying up front, but the edge cases are few and keeps expressions simple