Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert trusted actions list to data extension #18435

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
4 changes: 4 additions & 0 deletions actions/ql/lib/change-notes/2025-01-07-trusted-owner-ext.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: feature
---
* Trusted Action owner list can now be expanded using data extensions for `trustedActionsOwnerDataModel` on the query `actions/unpinned-tag`
7 changes: 7 additions & 0 deletions actions/ql/lib/codeql/actions/config/Config.qll
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,13 @@ predicate vulnerableActionsDataModel(
*/
predicate immutableActionsDataModel(string action) { Extensions::immutableActionsDataModel(action) }

/**
* MaD models for trusted actions owners
* Fields:
* - owner: owner name
*/
predicate trustedActionsOwnerDataModel(string owner) { Extensions::trustedActionsOwnerDataModel(owner) }

/**
* MaD models for untrusted git commands
* Fields:
Expand Down
5 changes: 5 additions & 0 deletions actions/ql/lib/codeql/actions/config/ConfigExtensions.qll
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ extensible predicate vulnerableActionsDataModel(
*/
extensible predicate immutableActionsDataModel(string action);

/**
* Holds for trusted Actions owners.
*/
extensible predicate trustedActionsOwnerDataModel(string owner);

/**
* Holds for git commands that may introduce untrusted data when called on an attacker controlled branch.
*/
Expand Down
8 changes: 8 additions & 0 deletions actions/ql/lib/ext/config/trusted_actions_owner.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
extensions:
- addsTo:
pack: codeql/actions-all
extensible: trustedActionsOwnerDataModel
data:
- ["actions"]
- ["github"]
- ["advanced-security"]
39 changes: 39 additions & 0 deletions actions/ql/src/Security/CWE-829/UnpinnedActionsTag.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,42 @@ Using a tag for a 3rd party Action that is not pinned to a commit can lead to ex

Pinning an action to a full length commit SHA is currently the only way to use a non-immutable action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. When selecting a SHA, you should verify it is from the action's repository and not a repository fork.


### Configuration

If there is an Action publisher that you trust, you can include the owner name/organization in a data extension model pack to add it to the allow list for this query. Adding owners to this list will prevent security alerts when using unpinned tags for Actions published by that owner.

#### Example

To allow any Action from the publisher `octodemo`, such as `octodemo/3rd-party-action`, follow these steps:

1. Create a data extension file `/models/trusted-owner.model.yml` with the following content:

```yaml
extensions:
- addsTo:
pack: codeql/actions-all
extensible: trustedActionsOwnerDataModel
data:
- ["octodemo"]
```

2. Create a model pack file `/codeql-pack.yml` with the following content:

```yaml
name: my-org/actions-extensions-model-pack
version: 0.0.0
library: true
extensionTargets:
codeql/actions-all: '*'
dataExtensions:
- models/**/*.yml
```

3. Ensure that the model pack is included in your CodeQL analysis.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step is not possible for default setup. I believe it's possible to just place the extensions file in .github/codeql/extensions in the current repository and have it automatically included in the run.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible to setup model packs with org level setup: Extending CodeQL coverage with CodeQL model packs in default setup ... added to references link at the bottom of the .md

I struggle with how verbose to make this guidance in the query markdown, feels like it would be better documented out in the (yet to be released) https://codeql.github.com/docs/codeql-language-guides/codeql-for-actions/

I will take a stab at adding some of the additional options here as a hint in the right direction

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm...that might be a better use of space to link out to the language guide (when it is available). I am a little concerned that if we load up the qhelp with lots of information around configuration, then autofix will get confused.

Presumably, the language docs are released along with the CLI release. So, timing would work out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point about autofix, I will shift gears and move these docs in that direction and consider deep linking via reference.

Copy link
Contributor Author

@felickz felickz Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the changelog a bit more verbose as a compromise.

Having an inventory of which queries are extensible via data extensions (and which scenarios) + including that in the query docs would be a larger effort that other queries would benefit from.

For future docs considerations:

Configuration

If there is an Action publisher that you trust, you can include the owner name/organization in a data extension model pack to add it to the allow list for this query. Adding owners to this list will prevent security alerts when using unpinned tags for Actions published by that owner.

Example

To allow any Action from the publisher octodemo, such as octodemo/3rd-party-action, follow these steps:

  1. Create a data extension file /models/trusted-owner.model.yml with the following content:

    extensions:
      - addsTo: 
          pack: codeql/actions-all
          extensible: trustedActionsOwnerDataModel 
        data:
          - ["octodemo"]
  2. Create a model pack file /codeql-pack.yml with the following content:

    name: my-org/actions-extensions-model-pack
    version: 0.0.0
    library: true
    extensionTargets:
      codeql/actions-all: '*'
    dataExtensions:
      - models/**/*.yml
  3. Ensure that the model pack is included in your CodeQL analysis.

By following these steps, you will add octodemo to the list of trusted Action publishers, and the query will no longer generate security alerts for unpinned tags from this publisher.

References


By following these steps, you will add `octodemo` to the list of trusted Action publishers, and the query will no longer generate security alerts for unpinned tags from this publisher.

## Examples

### Incorrect Usage
Expand All @@ -25,3 +61,6 @@ Pinning an action to a full length commit SHA is currently the only way to use a
## References

- [Using third-party actions](https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-third-party-actions)
- [Extending CodeQL coverage with CodeQL model packs in default setup](https://docs.github.com/en/code-security/code-scanning/managing-your-code-scanning-configuration/editing-your-configuration-of-default-setup#extending-codeql-coverage-with-codeql-model-packs-in-default-setup)
- [Creating and working with CodeQL packs](https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/creating-and-working-with-codeql-packs#creating-a-codeql-model-pack)

17 changes: 9 additions & 8 deletions actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed repo to nwo to be precise in the naming used here

Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,25 @@ import codeql.actions.security.UseOfUnversionedImmutableAction
bindingset[version]
private predicate isPinnedCommit(string version) { version.regexpMatch("^[A-Fa-f0-9]{40}$") }

bindingset[repo]
private predicate isTrustedOrg(string repo) {
repo.matches(["actions", "github", "advanced-security"] + "/%")
bindingset[nwo]
private predicate isTrustedOwner(string nwo) {
// Gets the segment before the first '/' in the name with owner(nwo) string
trustedActionsOwnerDataModel(nwo.substring(0, nwo.indexOf("/")))
}

from UsesStep uses, string repo, string version, Workflow workflow, string name
from UsesStep uses, string nwo, string version, Workflow workflow, string name
where
uses.getCallee() = repo and
uses.getCallee() = nwo and
uses.getEnclosingWorkflow() = workflow and
(
workflow.getName() = name
or
not exists(workflow.getName()) and workflow.getLocation().getFile().getBaseName() = name
) and
uses.getVersion() = version and
not isTrustedOrg(repo) and
not isTrustedOwner(nwo) and
not isPinnedCommit(version) and
not isImmutableAction(uses, repo)
not isImmutableAction(uses, nwo)
select uses.getCalleeNode(),
"Unpinned 3rd party Action '" + name + "' step $@ uses '" + repo + "' with ref '" + version +
"Unpinned 3rd party Action '" + name + "' step $@ uses '" + nwo + "' with ref '" + version +
"', not a pinned commit hash", uses, uses.toString()
Loading