Skip to content

Commit

Permalink
Rename DO- rules, inventory remaining rules missing from lv1 profile (#…
Browse files Browse the repository at this point in the history
…279)

* Add rule for OSPS-BR-01

* Rename DO- rules, inventory remaining rules missing from lv1 profile
  • Loading branch information
evankanderson authored Jan 24, 2025
1 parent 0fe35c1 commit cc41246
Show file tree
Hide file tree
Showing 16 changed files with 287 additions and 13 deletions.
16 changes: 11 additions & 5 deletions rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,19 @@ import (
"context"
"encoding/json"
"errors"
"github.com/rs/zerolog"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/reflect/protoreflect"
"gopkg.in/yaml.v3"
"io"
"net/http"
"os"
"path/filepath"
"strings"
"testing"

"github.com/rs/zerolog"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/reflect/protoreflect"
"gopkg.in/yaml.v3"

minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
rtengine "github.com/mindersec/minder/pkg/engine/v1/rtengine"
tkv1 "github.com/mindersec/minder/pkg/testkit/v1"
Expand All @@ -37,6 +38,8 @@ type RuleTest struct {
Entity EntityVersionWrapper `yaml:"entity"`
// Expect is the expected result of the test
Expect ExpectResult `yaml:"expect"`
// ErrorText is the expected error text of the test
ErrorText string `yaml:"error_text"`
// Git is the configuration for the git test
Git *GitTest `yaml:"git"`
// HTTP is the configuration for the HTTP test
Expand Down Expand Up @@ -164,6 +167,9 @@ func TestRuleTypes(t *testing.T) {
require.NoError(t, err)
} else {
require.Error(t, err)
if tc.ErrorText != "" {
require.Equal(t, strings.TrimSpace(tc.ErrorText), strings.TrimSpace(err.Error()))
}
}
})

Expand Down
33 changes: 27 additions & 6 deletions security-baseline/profiles/security-baseline-level-1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ context:
alert: "off"
remediate: "off"
repository:
# OSPS-AC-01: Require MFA for collaborators; depends on org entity
# OSPS-AC-02: Hosted GitHub has this by default

# OSPS-AC-03: Prevent overwriting git history
- name: osps-ac-03
type: osps-ac-03
Expand All @@ -15,14 +18,26 @@ repository:
- name: osps-ac-04
type: osps-ac-04
def: {}
# OSPS-DO-01: Projects has public discussion mechanisms
- name: osps-do-01
type: osps-do-01

# OSPS-BR-01: Prevent direct untrusted input in CI
- name: osps-br-01
type: osps-br-01
def: {}
# OSPS-DO-02: Enforce CONTRIBUTING file presence
- name: osps-do-02
type: osps-do-02
# OSPS-BR-03: Hosted GitHub has this by default
# OSPS-BR-09: GitHub hosted releases have this, check for links in release text?
# OSPS-BR-10: Not sure how to check this

# OSPS-DO-13: Need standardized format? Check for SUPPORT* in the meantime

# OSPS-GV-02: Projects has public discussion mechanisms
- name: osps-gv-02
type: osps-gv-02
def: {}
# OSPS-GV-03: Enforce CONTRIBUTING file presence
- name: osps-gv-03
type: osps-gv-03
def: {}

# OSPS-LE-02: Ensure OSI/FSF approved license
- name: osps-le-02
type: osps-le-02
Expand All @@ -31,6 +46,8 @@ repository:
- name: osps-le-03
type: osps-le-03
def: {}
# OSPS-LE-04: Check release object for LICENSE file

# OSPS-QA-01: Repository visibility check
- name: osps-qa-01
type: osps-qa-01
Expand All @@ -39,3 +56,7 @@ repository:
- name: osps-qa-02
type: osps-qa-02
def: {}

# OSPS-SA-02: Need standardized format? Also strange that this is lv1, but user guides are lv2

# OSPS-VM-05: Check for SECURITY.md or GitHub private vulnerability reporting
32 changes: 32 additions & 0 deletions security-baseline/rule-types/github/osps-br-01.test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
tests:
- name: No workflows
def: {}
params: {}
expect: "pass"
git:
repo_base: no_workflows
- name: Safe workflows
def: {}
params: {}
expect: pass
git:
repo_base: safe_workflows
- name: Unsafe checkout
def: {}
params: {}
expect: fail
error_text: |
evaluation failure: Evaluation failures:
- Workflow .github/workflows/pr_test.yaml has a dangerous trigger and checks out a pull request in job 'test'
- Workflow .github/workflows/pr_workflow.yaml has a dangerous trigger and checks out a pull request in job 'exec'
git:
repo_base: unsafe_checkout
- name: Script injection
def: {}
params: {}
expect: fail
error_text: |
evaluation failure: Evaluation failures:
- Workflow .github/workflows/pr_title.yaml has possible event script injection in step 0 of job 'check-title'
git:
repo_base: script_injection
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Empty repo

This repo does not have any workflows at all, which is a safe configuration.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Example test workflow for the GitHub Action feature
name: Test PR

on:
pull_request:
branches:
- main

jobs:
test:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Run tests
run: make test
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
on: [pull_request]

jobs:
check-title:
name: Check PR title
runs-on: ubuntu-latest
steps:

- name: Check PR title
run: |
title="${{ github.event.issue.title }}"
if [[ ! $title =~ ^.*:\ .*$ ]]; then
echo "Bad issue title"
exit 1
fi
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Example test workflow for the GitHub Action feature
name: Test PR

on:
pull_request_target:
branches:
- main

jobs:
test:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}

- name: Run tests
run: make test
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Example test workflow for the GitHub Action feature
name: Test PR

on: [workflow_run]

jobs:
exec:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.head.sha }}

- name: Run tests
run: make test
142 changes: 142 additions & 0 deletions security-baseline/rule-types/github/osps-br-01.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
version: v1
release_phase: alpha
type: rule-type
name: osps-br-01
display_name: Prevent untrusted input in continuous integration
short_failure_message: Untrusted input can subvert workflows
severity:
value: critical
context:
provider: github
description: |
This check determines whether the project's GitHub Action workflows
have dangerous code patterns. Some examples of these patterns are
untrusted code checkouts, logging github context and secrets, or use
of potentially untrusted inputs in scripts. The following patterns
are checked:
* Untrusted Code Checkout: This is the misuse of potentially
dangerous triggers. This checks if a pull_request_target or
workflow_run workflow trigger was used in conjunction with an
explicit pull request checkout. Workflows triggered with
pull_request_target / workflow_run have write permission to the
target repository and access to target repository secrets.
* Script Injection with Untrusted Context Variables: This pattern
detects whether a workflow's inline script may execute untrusted
input from attackers. Attackers can add their own content to
certain github context variables that are considered untrusted,
for example, github.event.issue.title. These values should not flow
directly into executable code.
guidance: |
Avoid using pull_request_target and workflow_run workflow triggers with
contributor-supplied code, following [GitHub's guidance](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)
[Quote or escape untrusted context variables](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections)
before using them in scripts.
def:
in_entity: repository
rule_schema: {}
ingest:
type: git
eval:
type: rego
rego:
type: constraints
def: |
package minder
import rego.v1
# Match both .yml and .yaml files
workflows := array.concat(file.ls_glob("./.github/workflows/*.yml"), file.ls_glob("./.github/workflows/*.yaml"))
dangerous_triggers = ["pull_request_target", "workflow_run"]
# Look for dangerous triggers combined with checkout of user-controlled code.
violations[{"msg": msg}] if {
some w
contents := file.read(workflows[w])
workflow := parse_yaml(contents)
events := events_set(workflow.on)
some event in events
event in dangerous_triggers
some job
some step
stepDef := workflow.jobs[job].steps[step]
startswith(stepDef.uses, "actions/checkout")
# This action is only dangerous if we check out the attacker-controlled branch
dangerous_ref(stepDef["with"].ref)
msg := sprintf("Workflow %s has a dangerous trigger and checks out a pull request in job '%s'", [workflows[w], job])
}
events_set(events) := object.keys(events) if {
is_object(events)
}
events_set(events) := events if {
is_array(events)
}
dangerous_ref(ref) if {
contains(ref, "github.event.pull_request")
}
dangerous_ref(ref) if {
contains(ref, "github.event.workflow_run")
}
# Look for possible script injections
violations[{"msg": msg}] if {
some w
contents := file.read(workflows[w])
workflow := parse_yaml(contents)
some job
some step
stepDef := workflow.jobs[job].steps[step]
stepDef.run
stepDef.run != "make test"
regexp := "\\${{(.*?)}}"
expansions := regex.find_all_string_submatch_n(regexp, stepDef.run, -1)
some expr in expansions
vulnerable_expansion(expr[1])
msg := sprintf("Workflow %s has possible event script injection in step %d of job '%s'", [workflows[w], step, job])
}
# Patterns from https://securitylab.github.com/resources/github-actions-untrusted-input/
vulnerable_patterns := {
"github.event.issue.title",
"github.event.issue.body",
"github.event.pull_request.title",
"github.event.pull_request.body",
"github.event.comment.body",
"github.event.review.body",
"github.event.pages.*.page_name",
"github.event.commits.*.message",
"github.event.head_commit.message",
"github.event.head_commit.author.email",
"github.event.head_commit.author.name",
"github.event.commits.*.author.email",
"github.event.commits.*.author.name",
"github.event.pull_request.head.ref",
"github.event.pull_request.head.label",
"github.event.pull_request.head.repo.default_branch",
"github.head_ref",
}
vulnerable_expansion(exp) if {
some match in vulnerable_patterns
matcher := concat("", ["*", match, "*"])
glob.match(matcher, null, exp)
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
version: v1
release_phase: alpha
type: rule-type
name: osps-do-01
name: osps-gv-02
display_name: The project has mechanisms for public discussion
short_failure_message: The project does not publicize mechanisms for public discussion.
severity:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
version: v1
release_phase: alpha
type: rule-type
name: osps-do-02
name: osps-gv-03
display_name: Contribution process is explained
short_failure_message: No CONTRIBUTING file or folder was found
severity:
Expand Down

0 comments on commit cc41246

Please sign in to comment.