Skip to content

Commit 3ffcc1e

Browse files
committed
Merge branch 'main' into cpp-access-paths-for-sources-and-sinks-3
2 parents 150d0a7 + 2876bb0 commit 3ffcc1e

145 files changed

Lines changed: 2822 additions & 1749 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/check-change-note.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
name: Check change note
22

33
permissions:
4+
contents: read
45
pull-requests: read
56

67
on:
7-
pull_request_target:
8+
pull_request:
89
types: [labeled, unlabeled, opened, synchronize, reopened, ready_for_review]
910
paths:
1011
- "*/ql/src/**/*.ql"
@@ -23,7 +24,7 @@ jobs:
2324
env:
2425
REPO: ${{ github.repository }}
2526
PULL_REQUEST_NUMBER: ${{ github.event.number }}
26-
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
27+
GH_TOKEN: ${{ github.token }}
2728
runs-on: ubuntu-latest
2829
steps:
2930

.github/workflows/labeler.yml

Lines changed: 136 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,144 @@
11
name: "Pull Request Labeler"
2+
23
on:
3-
- pull_request_target
4+
schedule:
5+
# Reconcile recently updated PRs promptly, including unapproved forks and
6+
# conflicted PRs for which pull_request workflows do not run.
7+
- cron: "7,22,37,52 * * * *"
8+
# Reconcile one stable shard of all open PRs each hour to recover from
9+
# delayed or missed scheduled runs.
10+
- cron: "12 * * * *"
11+
workflow_dispatch:
12+
inputs:
13+
pr_number:
14+
description: "Open pull request number to reconcile"
15+
required: true
16+
type: string
17+
18+
permissions: {}
419

5-
permissions:
6-
contents: read
7-
pull-requests: write
20+
concurrency:
21+
group: pull-request-labeler
22+
cancel-in-progress: false
823

924
jobs:
1025
triage:
26+
if: github.ref_name == github.event.repository.default_branch
1127
runs-on: ubuntu-latest
28+
timeout-minutes: 30
29+
permissions:
30+
contents: read
31+
pull-requests: write
1232
steps:
13-
- uses: actions/labeler@v4
14-
with:
15-
repo-token: "${{ secrets.GITHUB_TOKEN }}"
33+
- uses: actions/checkout@v5
34+
with:
35+
persist-credentials: false
36+
sparse-checkout: .github/labeler.yml
37+
sparse-checkout-cone-mode: false
38+
39+
- name: Collect pull requests to reconcile
40+
id: collect
41+
env:
42+
GH_TOKEN: ${{ github.token }}
43+
REPO: ${{ github.repository }}
44+
EVENT_NAME: ${{ github.event_name }}
45+
SCHEDULE: ${{ github.event.schedule }}
46+
REQUESTED_PR: ${{ inputs.pr_number }}
47+
run: |
48+
set -euo pipefail
49+
50+
if [ "$EVENT_NAME" = "workflow_dispatch" ]; then
51+
if [[ ! "$REQUESTED_PR" =~ ^[1-9][0-9]*$ ]]; then
52+
echo "Invalid pull request number: $REQUESTED_PR"
53+
exit 1
54+
fi
55+
56+
pr_json=$(gh api "repos/$REPO/pulls/$REQUESTED_PR")
57+
candidates=$(jq -c '[{
58+
number: .number,
59+
head_sha: .head.sha
60+
}]' <<<"$pr_json")
61+
else
62+
pulls_json=$(gh api --paginate \
63+
"repos/$REPO/pulls?state=open&sort=updated&direction=desc&per_page=100" |
64+
jq -cs 'add')
65+
66+
if [ "$SCHEDULE" = "12 * * * *" ]; then
67+
shard=$(( ($(date -u +%s) / 3600) % 6 ))
68+
candidates=$(jq -c --argjson shard "$shard" \
69+
'[.[] | select((.number % 6) == $shard) | {
70+
number: .number,
71+
head_sha: .head.sha
72+
}]' <<<"$pulls_json")
73+
else
74+
cutoff=$(date -u -d "1 hour ago" "+%Y-%m-%dT%H:%M:%SZ")
75+
# Hourly shards reconcile any candidates beyond this API budget.
76+
candidates=$(jq -c --arg cutoff "$cutoff" \
77+
'[.[] | select(.updated_at >= $cutoff) | {
78+
number: .number,
79+
head_sha: .head.sha
80+
}][0:100]' <<<"$pulls_json")
81+
fi
82+
fi
83+
84+
echo "Collected $(jq 'length' <<<"$candidates") pull request(s)."
85+
{
86+
echo "candidates<<EOF"
87+
echo "$candidates"
88+
echo "EOF"
89+
} >> "$GITHUB_OUTPUT"
90+
91+
- name: Validate pull request state
92+
id: validate
93+
env:
94+
GH_TOKEN: ${{ github.token }}
95+
REPO: ${{ github.repository }}
96+
CANDIDATES: ${{ steps.collect.outputs.candidates }}
97+
run: |
98+
set -euo pipefail
99+
100+
valid_numbers=()
101+
while IFS=$'\t' read -r pr_number expected_sha; do
102+
if [[ ! "$pr_number" =~ ^[1-9][0-9]*$ ]] ||
103+
[[ ! "$expected_sha" =~ ^[0-9a-f]{40}$ ]]; then
104+
echo "Skipping malformed pull request candidate."
105+
continue
106+
fi
107+
108+
if ! pr_json=$(gh api "repos/$REPO/pulls/$pr_number"); then
109+
echo "Pull request #$pr_number could not be fetched; skipping."
110+
continue
111+
fi
112+
113+
if ! jq -e \
114+
--arg repo "$REPO" \
115+
--arg sha "$expected_sha" \
116+
'.state == "open" and
117+
.base.repo.full_name == $repo and
118+
.head.sha == $sha and
119+
(.head.repo.full_name | type == "string")' \
120+
>/dev/null <<<"$pr_json"; then
121+
echo "Pull request #$pr_number changed or is no longer open; skipping."
122+
continue
123+
fi
124+
125+
valid_numbers+=("$pr_number")
126+
done < <(jq -r '.[] | [.number, .head_sha] | @tsv' <<<"$CANDIDATES")
127+
128+
if [ "${#valid_numbers[@]}" -eq 0 ]; then
129+
echo "has_prs=false" >> "$GITHUB_OUTPUT"
130+
exit 0
131+
fi
132+
133+
{
134+
echo "has_prs=true"
135+
echo "pr_numbers<<EOF"
136+
printf '%s\n' "${valid_numbers[@]}"
137+
echo "EOF"
138+
} >> "$GITHUB_OUTPUT"
139+
140+
- uses: actions/labeler@v4
141+
if: steps.validate.outputs.has_prs == 'true'
142+
with:
143+
repo-token: "${{ github.token }}"
144+
pr-number: ${{ steps.validate.outputs.pr_numbers }}

CODEOWNERS

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
/go/codeql-tools/ @github/codeql-go @github/code-scanning-language-coverage
1212
/go/downgrades/ @github/codeql-go @github/code-scanning-language-coverage
1313
/go/extractor/ @github/codeql-go @github/code-scanning-language-coverage
14-
/go/extractor-smoke-test/ @github/codeql-go @github/code-scanning-language-coverage
1514
/go/ql/test/extractor-tests/ @github/codeql-go @github/code-scanning-language-coverage
1615
/java/ @github/codeql-java
1716
/javascript/ @github/codeql-javascript

MODULE.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ use_repo(
302302
)
303303

304304
go_sdk = use_extension("@rules_go//go:extensions.bzl", "go_sdk")
305-
go_sdk.download(version = "1.26.6")
305+
go_sdk.download(version = "1.27.0")
306306

307307
go_deps = use_extension("@gazelle//:extensions.bzl", "go_deps")
308308
go_deps.from_file(go_mod = "//go/extractor:go.mod")
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
---
22
category: minorAnalysis
33
---
4-
* Added an option to `EnvironmentCheck` to become specified by a MaD model, otherwise it will continue as the default it previously was. Without adding models to `actions/ql/lib/ext/config/deployment_environment.yml` the behavior of every query will be unchanged. When models are added queries using `ControlCheck` may find more results in cases where an enironment is no longer a sufficient sanitizer.
4+
* Added an option to `EnvironmentCheck` to become specified by a MaD model, otherwise it will continue as the default it previously was. Without adding models to `actions/ql/lib/ext/config/deployment_environment.yml` the behavior of every query will be unchanged. When models are added queries using `ControlCheck` may find more results in cases where an environment is no longer a sufficient sanitizer.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Checks on actor fields read from the event payload (e.g. `github.event.pull_request.user.login`) now only count as protection for events whose payload actually populates that field. Previously, a condition such as `github.event.pull_request.user.login != 'name'` on a workflow triggered by `issues` events was treated as a protective check even though `github.event.pull_request` is not populated for `issues` events, which makes the condition vacuous. This change may result in more alerts for queries using the `ControlCheck` class.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: breaking
3+
---
4+
* Checks on actor fields read from the event payload (e.g. `github.event.pull_request.user.login`) were split out of `ActorIfCheck` into a new class `EventActorIfCheck`. The `ActorIfCheck` class now only covers `github.actor` and `github.triggering_actor`.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Checks on author association fields read from the event payload (e.g. `github.event.pull_request.author_association`) now only count as protection for events whose payload actually populates that field. Previously, a condition such as `github.event.pull_request.author_association != 'NONE'` on a workflow triggered by `issues` events was treated as a protective check even though `github.event.pull_request` is not populated for `issues` events, which makes the condition vacuous. This change may result in more alerts for queries using the `ControlCheck` class.

actions/ql/lib/codeql/actions/security/ControlChecks.qll

Lines changed: 73 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -314,17 +314,6 @@ class LabelIfCheck extends LabelCheck instanceof If {
314314

315315
class ActorIfCheck extends ActorCheck instanceof If {
316316
ActorIfCheck() {
317-
// eg: github.event.pull_request.user.login == 'admin'
318-
exists(
319-
normalizeExpr(this.getCondition())
320-
.regexpFind([
321-
"\\bgithub\\.event\\.pull_request\\.user\\.login\\b",
322-
"\\bgithub\\.event\\.head_commit\\.author\\.name\\b",
323-
"\\bgithub\\.event\\.commits.*\\.author\\.name\\b",
324-
"\\bgithub\\.event\\.sender\\.login\\b"
325-
], _, _)
326-
)
327-
or
328317
// eg: github.actor == 'admin'
329318
// eg: github.triggering_actor == 'admin'
330319
exists(
@@ -335,6 +324,51 @@ class ActorIfCheck extends ActorCheck instanceof If {
335324
}
336325
}
337326

327+
/**
328+
* Gets a regular expression matching a condition on an actor field that is
329+
* only populated for events whose payload contains the `context_prefix` context.
330+
*/
331+
private string eventPayloadActorFieldRegex(string context_prefix) {
332+
context_prefix = "github.event.pull_request" and
333+
result = "\\bgithub\\.event\\.pull_request\\.user\\.login\\b"
334+
or
335+
context_prefix = "github.event.head_commit" and
336+
result = "\\bgithub\\.event\\.head_commit\\.author\\.name\\b"
337+
or
338+
context_prefix = "github.event.commits" and
339+
result = "\\bgithub\\.event\\.commits.*\\.author\\.name\\b"
340+
or
341+
context_prefix = "github.event.sender" and
342+
result = "\\bgithub\\.event\\.sender\\.login\\b"
343+
}
344+
345+
/** An If node that checks an actor field from the event payload */
346+
class EventActorIfCheck extends ActorCheck instanceof If {
347+
string context_prefix;
348+
349+
EventActorIfCheck() {
350+
// eg: github.event.pull_request.user.login == 'admin'
351+
exists(
352+
normalizeExpr(this.getCondition())
353+
.regexpFind(eventPayloadActorFieldRegex(context_prefix), _, _)
354+
)
355+
}
356+
357+
override predicate protectsCategoryAndEvent(string category, string event) {
358+
ActorCheck.super.protectsCategoryAndEvent(category, event) and
359+
(
360+
// the `sender` object is part of every webhook event payload
361+
context_prefix = "github.event.sender"
362+
or
363+
// other actor fields only restrict events whose payload populates them.
364+
// eg: `github.event.pull_request.user.login` cannot restrict the actor
365+
// of an `issues` event since `github.event.pull_request` is not
366+
// populated there, which makes the condition vacuous
367+
contextTriggerDataModel(event, context_prefix)
368+
)
369+
}
370+
}
371+
338372
class PullRequestTargetRepositoryIfCheck extends RepositoryCheck instanceof If {
339373
PullRequestTargetRepositoryIfCheck() {
340374
// eg: github.event.pull_request.head.repo.full_name == github.repository
@@ -374,16 +408,37 @@ class WorkflowRunRepositoryIfCheck extends RepositoryCheck instanceof If {
374408
}
375409
}
376410

411+
/**
412+
* Gets a regular expression matching a condition on an author association field
413+
* that is only populated for events whose payload contains the `context_prefix`
414+
* context.
415+
*/
416+
private string eventPayloadAssociationFieldRegex(string context_prefix) {
417+
context_prefix = "github.event.comment" and
418+
result = "\\bgithub\\.event\\.comment\\.author_association\\b"
419+
or
420+
context_prefix = "github.event.issue" and
421+
result = "\\bgithub\\.event\\.issue\\.author_association\\b"
422+
or
423+
context_prefix = "github.event.pull_request" and
424+
result = "\\bgithub\\.event\\.pull_request\\.author_association\\b"
425+
}
426+
377427
class AssociationIfCheck extends AssociationCheck instanceof If {
428+
string context_prefix;
429+
378430
AssociationIfCheck() {
379431
// eg: contains(fromJson('["MEMBER", "OWNER"]'), github.event.comment.author_association)
380-
normalizeExpr(this.getCondition())
381-
.splitAt("\n")
382-
.regexpMatch([
383-
".*\\bgithub\\.event\\.comment\\.author_association\\b.*",
384-
".*\\bgithub\\.event\\.issue\\.author_association\\b.*",
385-
".*\\bgithub\\.event\\.pull_request\\.author_association\\b.*",
386-
])
432+
exists(
433+
normalizeExpr(this.getCondition())
434+
.regexpFind(eventPayloadAssociationFieldRegex(context_prefix), _, _)
435+
)
436+
}
437+
438+
override predicate protectsCategoryAndEvent(string category, string event) {
439+
AssociationCheck.super.protectsCategoryAndEvent(category, event) and
440+
// association fields only restrict events whose payload populates them
441+
contextTriggerDataModel(event, context_prefix)
387442
}
388443
}
389444

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
on:
2+
pull_request_target:
3+
types: [opened]
4+
5+
jobs:
6+
# The `if:` condition checks an actor field that is populated for
7+
# `pull_request_target` events, so the injectable step is protected.
8+
valid-actor-check:
9+
runs-on: ubuntu-latest
10+
if: github.event.pull_request.user.login == 'trusted-user'
11+
steps:
12+
- run: echo '${{ github.event.pull_request.title }}'

0 commit comments

Comments
 (0)