From 023d72f5b813e344683ebbef1c180ac7cd263f83 Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 25 Aug 2025 17:16:44 -0400 Subject: [PATCH 1/7] get_key for maps where the key contains dashes :) --- lib/claws/base_rule.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/claws/base_rule.rb b/lib/claws/base_rule.rb index 02763da..86b28a1 100644 --- a/lib/claws/base_rule.rb +++ b/lib/claws/base_rule.rb @@ -14,6 +14,7 @@ def ctx # rubocop:disable Metrics/AbcSize endswith: ->(string, needle) { string.to_s.end_with? needle }, difference: ->(arr1, arr2) { arr1.difference arr2 }, intersection: ->(arr1, arr2) { arr1.intersection arr2 }, + get_key: ->(arr, key) { arr.fetch(key, nil) }, count: ->(n) { n.length } } ) From 82c4640eb6fc0a5fd99da209784f8f5113353bae Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 25 Aug 2025 17:17:13 -0400 Subject: [PATCH 2/7] detections for static credentials --- lib/claws/rule.rb | 1 + .../rule/checkout_with_static_credentials.rb | 38 ++++ ...heckout_without_static_credentials_spec.rb | 163 ++++++++++++++++++ 3 files changed, 202 insertions(+) create mode 100644 lib/claws/rule/checkout_with_static_credentials.rb create mode 100644 spec/claws/rule/checkout_without_static_credentials_spec.rb diff --git a/lib/claws/rule.rb b/lib/claws/rule.rb index e483bfe..fcd0ba5 100644 --- a/lib/claws/rule.rb +++ b/lib/claws/rule.rb @@ -11,3 +11,4 @@ require "claws/rule/command_injection" require "claws/rule/bulk_permissions" require "claws/rule/shellcheck" +require "claws/rule/checkout_with_static_credentials" diff --git a/lib/claws/rule/checkout_with_static_credentials.rb b/lib/claws/rule/checkout_with_static_credentials.rb new file mode 100644 index 0000000..e7694d1 --- /dev/null +++ b/lib/claws/rule/checkout_with_static_credentials.rb @@ -0,0 +1,38 @@ +module Claws + module Rule + class CheckoutWithStaticCredentials < BaseRule + description <<~DESC + Avoid using static credentials like deploy keys, SSH keys, or personal access + tokens to clone other repositories. Static credentials can be tricky to audit + and rotate, making them risky to hold onto, especially in the event of an + incident where they may be leaked. + + Either grant your repository access directly to other repositories, or use a + Github App to generate a short lived access token. + + For more information: + https://github.com/betterment/claws/blob/main/README.md#checkoutwithstaticcredentials + DESC + + on_step %( + $step.meta.action.name == "actions/checkout" && + ( + get_key($step.with, "ssh-key") =~ ".*secrets..*" || + get_key($step.with, "ssh-key") =~ ".*env..*" || + get_key($step.with, "ssh-key") =~ ".*vars..*" || + get_key($step.with, "ssh-key") =~ ".*-----BEGIN.*" + ) + ), highlight: "with.ssh-key" + + on_step %( + $step.meta.action.name == "actions/checkout" && + ( + get_key($step.with, "token") =~ ".*secrets.*" || + get_key($step.with, "token") =~ ".*env..*" || + get_key($step.with, "token") =~ ".*vars..*" || + get_key($step.with, "token") =~ "gh[a-z]_.*" + ) + ), highlight: "with.token" + end + end +end diff --git a/spec/claws/rule/checkout_without_static_credentials_spec.rb b/spec/claws/rule/checkout_without_static_credentials_spec.rb new file mode 100644 index 0000000..14439b2 --- /dev/null +++ b/spec/claws/rule/checkout_without_static_credentials_spec.rb @@ -0,0 +1,163 @@ +RSpec.describe Claws::Rule::CheckoutWithStaticCredentials do + before do + load_detection + end + + context "with default configuration" do + it "flags a static ssh key via secrets" do + violations = analyze(<<~YAML) + on: push + + jobs: + checkout: + runs-on: ubuntu + steps: + - uses: actions/checkout@v5 + with: + repository: foo-corp/test-action + ssh-key: ${{ secrets.deploy_key }} + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(10) + expect(violations[0].name).to eq("CheckoutWithStaticCredentials") + end + + it "flags a static ssh key via repo/org vars" do + violations = analyze(<<~YAML) + on: push + + jobs: + checkout: + runs-on: ubuntu + steps: + - uses: actions/checkout@v5 + with: + repository: foo-corp/test-action + ssh-key: ${{ vars.deploy_key }} + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(10) + expect(violations[0].name).to eq("CheckoutWithStaticCredentials") + end + + it "flags a static ssh key via env vars" do + violations = analyze(<<~YAML) + on: push + + jobs: + checkout: + runs-on: ubuntu + steps: + - uses: actions/checkout@v5 + with: + repository: foo-corp/test-action + token: ${{ env.deploy_key }} + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(10) + expect(violations[0].name).to eq("CheckoutWithStaticCredentials") + end + + it "flags a static ssh key via hardcoded string" do + violations = analyze(<<~YAML) + on: push + + jobs: + checkout: + runs-on: ubuntu + steps: + - uses: actions/checkout@v5 + with: + repository: foo-corp/test-action + ssh-key: | + -----BEGIN OPENSSH PRIVATE KEY----- + sike... you thought + -----END OPENSSH PRIVATE KEY----- + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(10) + expect(violations[0].name).to eq("CheckoutWithStaticCredentials") + end + + it "flags a static PAT stored in secrets" do + violations = analyze(<<~YAML) + on: push + + jobs: + checkout: + runs-on: ubuntu + steps: + - uses: actions/checkout@v5 + with: + repository: foo-corp/test-action + token: ${{ secrets.secret_pat }} + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(10) + expect(violations[0].name).to eq("CheckoutWithStaticCredentials") + end + + it "flags a static PAT stored in repo/org vars" do + violations = analyze(<<~YAML) + on: push + + jobs: + checkout: + runs-on: ubuntu + steps: + - uses: actions/checkout@v5 + with: + repository: foo-corp/test-action + token: ${{ vars.secret_pat }} + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(10) + expect(violations[0].name).to eq("CheckoutWithStaticCredentials") + end + + it "flags a static PAT stored in env vars" do + violations = analyze(<<~YAML) + on: push + + jobs: + checkout: + runs-on: ubuntu + steps: + - uses: actions/checkout@v5 + with: + repository: foo-corp/test-action + token: ${{ env.some_pat }} + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(10) + expect(violations[0].name).to eq("CheckoutWithStaticCredentials") + end + + it "flags a static PAT via hardcoded string" do + violations = analyze(<<~YAML) + on: push + + jobs: + checkout: + runs-on: ubuntu + steps: + - uses: actions/checkout@v5 + with: + repository: foo-corp/test-action + token: "ghp_some_bogus_string" + YAML + + expect(violations.count).to eq(1) + expect(violations[0].line).to eq(10) + expect(violations[0].name).to eq("CheckoutWithStaticCredentials") + end + + end +end From d2b16700f284c37559251f38eec29191371aca9e Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 25 Aug 2025 17:17:26 -0400 Subject: [PATCH 3/7] documentation --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index a12cbaa..d4f681e 100644 --- a/README.md +++ b/README.md @@ -280,6 +280,18 @@ Shellcheck is a great tool for dealing with bugs or otherwise unintended effects This rule flags workflows that request write access to specific unusual permissions. While this rule cannot flag how these permissions are exercised, it serves as a warning to code reviewers that if these permissions are requested, the way they are used should be scrutinized. A reviewer may find that a permission is left over from testing and no longer needed, or that a specific permission was never needed. +### CheckoutWithStaticCredentials + +This rule flags any uses of `actions/checkout` using static credentials. Using static credentials can pose a risk because these credentials are typically not auditable and can be tricky to rotate. In the event of an incident where they are leaked, incident response to determine the scope of impact may be tough. + +Where possible, the default `$GITHUB_TOKEN` should be used. Its settings can be configured directly from within the workflow. Check [the official documentation](https://docs.github.com/en/actions/tutorials/authenticate-with-github_token#modifying-the-permissions-for-the-github_token) for more information on how to do this. + +If you are using a deploy key via SSH to access a package or otherwise an artifact from another repository, you can instead configure the repository to grant it access explicitly to that other repository. This will give the default `$GITHUB_TOKEN` access to that repository without needing to use a deploy key. To learn more about this, [check out the official documentation](https://docs.github.com/en/packages/learn-github-packages/configuring-a-packages-access-control-and-visibility). This is safer than a static deploy key because the credential is short lived and access can be audited. + +If you need a Github Token to perform some authenticated action where the default `$GITHUB_TOKEN` doesn't do what you need, consider setting up a Github App and using [`actions/create-github-app-token`](https://github.com/actions/create-github-app-token). This will generate a short lived access token, and using an app creates a useful audit trail for what this access token can actually do. Then, use this token in just the build steps where it's needed. + +In some cases, a static access token or deploy key may still be necessary, especially for APIs that are not yet supported by Github App Tokens. In these cases, make sure to limit the scope of the access token to the bare minimum necessary to function. + ### UnapprovedRunners This rule flags workflows that use runners that they might not need or should not use. This can come in handy when an organization has available self hosted or otherwise expensive runners but wants to be particular about when they're used. From 8828bbbb81d8ebbc5dd55e1741131adf40ba339a Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 25 Aug 2025 17:21:14 -0400 Subject: [PATCH 4/7] stray newline --- spec/claws/rule/checkout_without_static_credentials_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/claws/rule/checkout_without_static_credentials_spec.rb b/spec/claws/rule/checkout_without_static_credentials_spec.rb index 14439b2..d85d5f1 100644 --- a/spec/claws/rule/checkout_without_static_credentials_spec.rb +++ b/spec/claws/rule/checkout_without_static_credentials_spec.rb @@ -158,6 +158,5 @@ expect(violations[0].line).to eq(10) expect(violations[0].name).to eq("CheckoutWithStaticCredentials") end - end end From abf9259c365fa94a335818e9f4ea710e240f9028 Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 25 Aug 2025 17:24:27 -0400 Subject: [PATCH 5/7] git mv spec/claws/rule/checkout_with{out,}_static_credentials_spec.rb --- ...edentials_spec.rb => checkout_with_static_credentials_spec.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/claws/rule/{checkout_without_static_credentials_spec.rb => checkout_with_static_credentials_spec.rb} (100%) diff --git a/spec/claws/rule/checkout_without_static_credentials_spec.rb b/spec/claws/rule/checkout_with_static_credentials_spec.rb similarity index 100% rename from spec/claws/rule/checkout_without_static_credentials_spec.rb rename to spec/claws/rule/checkout_with_static_credentials_spec.rb From bd071a967f53dcf32ddb6da161472647c53e602b Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 25 Aug 2025 17:38:53 -0400 Subject: [PATCH 6/7] more precise checks + negative test case --- .../rule/checkout_with_static_credentials.rb | 12 +++++----- .../checkout_with_static_credentials_spec.rb | 24 +++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/lib/claws/rule/checkout_with_static_credentials.rb b/lib/claws/rule/checkout_with_static_credentials.rb index e7694d1..2797fc4 100644 --- a/lib/claws/rule/checkout_with_static_credentials.rb +++ b/lib/claws/rule/checkout_with_static_credentials.rb @@ -17,9 +17,9 @@ class CheckoutWithStaticCredentials < BaseRule on_step %( $step.meta.action.name == "actions/checkout" && ( - get_key($step.with, "ssh-key") =~ ".*secrets..*" || - get_key($step.with, "ssh-key") =~ ".*env..*" || - get_key($step.with, "ssh-key") =~ ".*vars..*" || + get_key($step.with, "ssh-key") =~ "{{.*secrets\..*" || + get_key($step.with, "ssh-key") =~ "{{.*env\..*" || + get_key($step.with, "ssh-key") =~ "{{.*vars\..*" || get_key($step.with, "ssh-key") =~ ".*-----BEGIN.*" ) ), highlight: "with.ssh-key" @@ -27,9 +27,9 @@ class CheckoutWithStaticCredentials < BaseRule on_step %( $step.meta.action.name == "actions/checkout" && ( - get_key($step.with, "token") =~ ".*secrets.*" || - get_key($step.with, "token") =~ ".*env..*" || - get_key($step.with, "token") =~ ".*vars..*" || + get_key($step.with, "token") =~ "{{.*secrets\..*" || + get_key($step.with, "token") =~ "{{.*env\..*" || + get_key($step.with, "token") =~ "{{.*vars\..*" || get_key($step.with, "token") =~ "gh[a-z]_.*" ) ), highlight: "with.token" diff --git a/spec/claws/rule/checkout_with_static_credentials_spec.rb b/spec/claws/rule/checkout_with_static_credentials_spec.rb index d85d5f1..5427282 100644 --- a/spec/claws/rule/checkout_with_static_credentials_spec.rb +++ b/spec/claws/rule/checkout_with_static_credentials_spec.rb @@ -158,5 +158,29 @@ expect(violations[0].line).to eq(10) expect(violations[0].name).to eq("CheckoutWithStaticCredentials") end + + it "doesn't flag an access token generated using a github app" do + violations = analyze(<<~YAML) + on: push + + jobs: + checkout: + runs-on: ubuntu + steps: + - name: Create GitHub App installation token + id: app-token + uses: actions/create-github-app-token@v2 + with: + app-id: ${{ vars.APP_ID }} + private-key: ${{ secrets.APP_PRIVATE_KEY }} + + - uses: actions/checkout@v5 + with: + repository: foo-corp/test-action + token: ${{ steps.app-token.outputs.token }} + YAML + + expect(violations.count).to eq(0) + end end end From 6e92dc247b99d31ce30a19ffe0d42f0f088ffd6f Mon Sep 17 00:00:00 2001 From: Omar <47008591+6f6d6172@users.noreply.github.com> Date: Mon, 25 Aug 2025 17:53:22 -0400 Subject: [PATCH 7/7] get_key should check that the input map isn't nil --- lib/claws/base_rule.rb | 2 +- spec/claws/workflow_spec.rb | 54 +++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/lib/claws/base_rule.rb b/lib/claws/base_rule.rb index 86b28a1..4706a57 100644 --- a/lib/claws/base_rule.rb +++ b/lib/claws/base_rule.rb @@ -14,7 +14,7 @@ def ctx # rubocop:disable Metrics/AbcSize endswith: ->(string, needle) { string.to_s.end_with? needle }, difference: ->(arr1, arr2) { arr1.difference arr2 }, intersection: ->(arr1, arr2) { arr1.intersection arr2 }, - get_key: ->(arr, key) { arr.fetch(key, nil) }, + get_key: ->(arr, key) { (arr || {}).fetch(key, nil) }, count: ->(n) { n.length } } ) diff --git a/spec/claws/workflow_spec.rb b/spec/claws/workflow_spec.rb index 047e4d5..3f79ea7 100644 --- a/spec/claws/workflow_spec.rb +++ b/spec/claws/workflow_spec.rb @@ -78,4 +78,58 @@ expect(BaseRule.parse_rule("$step.with.type_float == 1.2").eval_with(values:)).to eq true end end + + context "built in function - get_key" do + it "extracts the key from a map" do + workflow = described_class.load(<<~YAML) + on: + pull_request + + jobs: + deploy: + steps: + - name: checkout + uses: actions/checkout@v6 + with: + key: value + YAML + + values = { workflow:, job: workflow.jobs["deploy"], step: workflow.jobs["deploy"]["steps"][0] } + expect(BaseRule.parse_rule('get_key($step.with, "key")').eval_with(values:)).to eq "value" + end + + it "returns nil if the key isn't found" do + workflow = described_class.load(<<~YAML) + on: + pull_request + + jobs: + deploy: + steps: + - name: checkout + uses: actions/checkout@v6 + with: + key: value + YAML + + values = { workflow:, job: workflow.jobs["deploy"], step: workflow.jobs["deploy"]["steps"][0] } + expect(BaseRule.parse_rule('get_key($step.with, "nonexistent")').eval_with(values:)).to eq nil + end + + it "returns nil if the input map is nil" do + workflow = described_class.load(<<~YAML) + on: + pull_request + + jobs: + deploy: + steps: + - name: checkout + uses: actions/checkout@v6 + YAML + + values = { workflow:, job: workflow.jobs["deploy"], step: workflow.jobs["deploy"]["steps"][0] } + expect(BaseRule.parse_rule('get_key($step.with, "nonexistent")').eval_with(values:)).to eq nil + end + end end