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. diff --git a/lib/claws/base_rule.rb b/lib/claws/base_rule.rb index 02763da..4706a57 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 } } ) 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..2797fc4 --- /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_with_static_credentials_spec.rb b/spec/claws/rule/checkout_with_static_credentials_spec.rb new file mode 100644 index 0000000..5427282 --- /dev/null +++ b/spec/claws/rule/checkout_with_static_credentials_spec.rb @@ -0,0 +1,186 @@ +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 + + 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 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