Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions lib/claws/base_rule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
)
Expand Down
1 change: 1 addition & 0 deletions lib/claws/rule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
38 changes: 38 additions & 0 deletions lib/claws/rule/checkout_with_static_credentials.rb
Original file line number Diff line number Diff line change
@@ -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
186 changes: 186 additions & 0 deletions spec/claws/rule/checkout_with_static_credentials_spec.rb
Original file line number Diff line number Diff line change
@@ -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)
Comment thread
6f6d6172 marked this conversation as resolved.
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
54 changes: 54 additions & 0 deletions spec/claws/workflow_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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