Skip to content

Commit c29f3a7

Browse files
author
Alvaro Muñoz
authored
Merge pull request #21 from GitHubSecurityLab/refactor_env_access
refactor env access
2 parents 1458434 + 98f3a1e commit c29f3a7

File tree

5 files changed

+359
-66
lines changed

5 files changed

+359
-66
lines changed

ql/lib/codeql/actions/Ast.qll

Lines changed: 19 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,21 @@ class AstNode instanceof YamlNode {
2323
class Statement extends AstNode {
2424
/** Gets the workflow that this job is a part of. */
2525
WorkflowStmt getEnclosingWorkflowStmt() { this = result.getAChildNode*() }
26+
27+
/**
28+
* Gets a environment variable expression by name in the scope of the current step.
29+
*/
30+
Expression getEnvExpr(string name) {
31+
exists(Actions::Env env |
32+
env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
33+
|
34+
env.(Actions::StepEnv).getStep().getAChildNode*() = this
35+
or
36+
env.(Actions::JobEnv).getJob().getAChildNode*() = this
37+
or
38+
env.(Actions::WorkflowEnv).getWorkflow().getAChildNode*() = this
39+
)
40+
}
2641
}
2742

2843
/**
@@ -212,8 +227,6 @@ abstract class UsesExpr extends Expression {
212227
abstract string getVersion();
213228

214229
abstract Expression getArgumentExpr(string key);
215-
216-
abstract Expression getEnvExpr(string name);
217230
}
218231

219232
/**
@@ -234,26 +247,6 @@ class StepUsesExpr extends StepStmt, UsesExpr {
234247
result = with.lookup(key)
235248
)
236249
}
237-
238-
/**
239-
* Gets a environment variable expression by name in the scope of the current step.
240-
*/
241-
override Expression getEnvExpr(string name) {
242-
exists(Actions::StepEnv env |
243-
env.getStep() = this and
244-
env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
245-
)
246-
or
247-
exists(Actions::JobEnv env |
248-
env.getJob() = this.getJobStmt() and
249-
env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
250-
)
251-
or
252-
exists(Actions::WorkflowEnv env |
253-
env.getWorkflow() = this.getJobStmt().getEnclosingWorkflowStmt() and
254-
env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
255-
)
256-
}
257250
}
258251

259252
/**
@@ -302,23 +295,6 @@ class JobUsesExpr extends UsesExpr instanceof YamlMapping {
302295
override Expression getArgumentExpr(string key) {
303296
this.(YamlMapping).lookup("with").(YamlMapping).lookup(key) = result
304297
}
305-
306-
/**
307-
* Gets a environment variable expression by name in the scope of the current node.
308-
*/
309-
override Expression getEnvExpr(string name) {
310-
this.(YamlMapping).lookup("env").(YamlMapping).lookup(name) = result
311-
or
312-
exists(Actions::JobEnv env |
313-
env.getJob() = this.getJobStmt() and
314-
env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
315-
)
316-
or
317-
exists(Actions::WorkflowEnv env |
318-
env.getWorkflow() = this.getJobStmt().getEnclosingWorkflowStmt() and
319-
env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
320-
)
321-
}
322298
}
323299

324300
/**
@@ -332,26 +308,6 @@ class RunExpr extends StepStmt, Expression {
332308
Expression getScriptExpr() { result = scriptExpr }
333309

334310
string getScript() { result = scriptExpr.getValue() }
335-
336-
/**
337-
* Gets a environment variable expression by name in the scope of the current node.
338-
*/
339-
Expression getEnvExpr(string name) {
340-
exists(Actions::StepEnv env |
341-
env.getStep() = this and
342-
env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
343-
)
344-
or
345-
exists(Actions::JobEnv env |
346-
env.getJob() = this.getJobStmt() and
347-
env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
348-
)
349-
or
350-
exists(Actions::WorkflowEnv env |
351-
env.getWorkflow() = this.getJobStmt().getEnclosingWorkflowStmt() and
352-
env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
353-
)
354-
}
355311
}
356312

357313
/**
@@ -523,11 +479,9 @@ class EnvCtxAccessExpr extends CtxAccessExpr {
523479
override string getFieldName() { result = fieldName }
524480

525481
override Expression getRefExpr() {
526-
result.getLocation().getFile() = this.getLocation().getFile() and
527-
exists(JobUsesExpr s | s.getEnvExpr(fieldName) = result)
528-
or
529-
exists(StepUsesExpr s | s.getEnvExpr(fieldName) = result)
530-
or
531-
exists(RunExpr s | s.getEnvExpr(fieldName) = result)
482+
exists(Statement s |
483+
s.getEnvExpr(fieldName) = result and
484+
s.getAChildNode*() = this
485+
)
532486
}
533487
}

ql/src/Security/CWE-094/UntrustedCheckout.ql

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ class ActorCheckStmt extends IfStmt {
2525
* An If node that contains a `label` check
2626
*/
2727
class LabelCheckStmt extends IfStmt {
28-
LabelCheckStmt() { this.getCondition().regexpMatch(".*github\\.event\\.pull_request\\.labels.*") }
28+
LabelCheckStmt() {
29+
this.getCondition().regexpMatch(".*github\\.event\\.pull_request\\.labels.*") or
30+
this.getCondition().regexpMatch(".*github\\.event\\.label\\.name.*")
31+
}
2932
}
3033

3134
from WorkflowStmt w, JobStmt job, StepUsesExpr checkoutStep
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
# Issues_workflow.yaml (https://github.com/Bughalla/dynamods_dynamo/blob/1c1d3e29ee9bca81b43d78f22bf953100ef67009/.github/workflows/Issues_workflow.yaml#L128-L128)
2+
name: Issue Workflow
3+
on:
4+
issues:
5+
types: [opened,edited]
6+
jobs:
7+
#This job will check the issue to determine if it should be moved to a different repository
8+
redirectIssue:
9+
name: Check for issue transfer
10+
runs-on: ubuntu-latest
11+
env:
12+
#The 'content_analysis_response' variable is used to store the script response on step one,
13+
#and then checked on step two to know if adding any labels is necessary.
14+
#The initial 'undefined' value will be overridden when the script runs.
15+
content_analysis_response: undefined
16+
ISSUE_TITLE: ${{github.event.issue.title}}
17+
ISSUE_BODY: ${{github.event.issue.body}}
18+
outputs:
19+
result: ${{env.content_analysis_response}}
20+
steps:
21+
- uses: actions/checkout@v4
22+
23+
#Detect if the issue_title follows the regex expression
24+
- name: Check Issue Title
25+
uses: actions-ecosystem/action-regex-match@v2
26+
id: regex-match
27+
with:
28+
text: ${{github.event.issue.title}}
29+
regex: '^[A-Za-z0-9 _.]*$'
30+
flags: g
31+
32+
#If the regex output is '' means that the issue title contains special chars
33+
- name: Exit Job
34+
if: ${{ steps.regex-match.outputs.match == '' }}
35+
run: |
36+
echo "Bad Issue Title Format"
37+
exit 1
38+
39+
#Remove the " character in the issue title and replaced with -
40+
- name: Remove conflicting chars
41+
uses: frabert/[email protected]
42+
id: remove_quotations
43+
with:
44+
pattern: "\""
45+
string: ${{env.ISSUE_TITLE}}
46+
replace-with: '-'
47+
flags: g
48+
49+
#According to the issue_title returns a specific label
50+
- name: Check Information
51+
id: check-info
52+
env:
53+
ISSUE_TITLE_PARSED: ${{steps.remove_quotations.outputs.replaced}}
54+
run: |
55+
echo "content_analysis_response=$(pwsh .\\.github\\scripts\\title_analyzer.ps1)" >> $GITHUB_ENV
56+
57+
#labels the issue based in the text returned in content_analysis_response var
58+
- name: Label issue
59+
if: env.content_analysis_response != 'Valid'
60+
#Uses DYNAMOBOTTOKEN to allow interaction between repos
61+
run: |
62+
curl -v -u admin:${{ secrets.DYNAMOBOTTOKEN }} -d '{"labels": ["${{env.content_analysis_response}}"]}' ${{ github.event.issue.url }}/labels
63+
64+
#This job will scan the issue content to determing if more information is needed and act acordingly
65+
#Will only run if the "redirectIssue" job outputted a 'Valid' result
66+
checkIssueInformation:
67+
if: needs.redirectIssue.outputs.result == 'Valid'
68+
name: Check for missing information
69+
#Wait for the previous job to finish as it needs its output
70+
needs: redirectIssue
71+
runs-on: ubuntu-latest
72+
env:
73+
#The 'analysis_response' variable is used to store the script response on step one,
74+
#and then checked on step two to know if adding the label and comment is necessary.
75+
#The initial 'undefined' value will be overridden when the script runs.
76+
analysis_response: undefined
77+
#Greetings for valid issues
78+
greetings_comment: "Thank you for submitting the issue to us. We are sorry to
79+
see you get stuck with your workflow. While waiting for our team member to respond,
80+
please feel free to browse our forum at https://forum.dynamobim.com/ for more Dynamo related information."
81+
#Comment intro
82+
comment_intro: "Hello ${{ github.actor }}, thank you for submitting this issue!
83+
We are super excited that you want to help us make Dynamo all that it can be."
84+
#issue_coment holds the comment format, while the missing information will be provided by analysis_response
85+
needs_more_info_comment: "However, we need some more information in order for the Dynamo
86+
team to investigate any further.\\n\\n"
87+
#comment to be used if the issue is closed due to the template being empty
88+
close_issue_comment: "However, given that there has been no additional information added,
89+
this issue will be closed for now. Please reopen and provide additional
90+
information if you wish the Dynamo team to investigate further.\\n\\n"
91+
#Info asked from the user in bot comments
92+
info_needed: "Additional information:\\n
93+
- Filling in of the provided Template (What did you do, What did you expect to see,
94+
What did you see instead, What packages or external references (if any) were used)\\n
95+
- Attaching the Stack Trace (Error message that shows up when Dynamo crashes - You can copy and paste this into the Github Issue)\\n
96+
- Upload a .DYN file that showcases the issue in action and any additional needed files, such as Revit
97+
(Note: If you cannot share a project, you can recreate this in a quick mock-up file)\\n
98+
- Upload a Screenshot of the error messages you see (Hover over the offending node and showcase
99+
said errors message in the screenshot)\\n
100+
- Reproducible steps on how to create the error in question."
101+
#Text to ask for specific missing information (complemented by the analysis response)
102+
specific_info: "Can you please fill in the following to the best of your ability:"
103+
#template file name
104+
template: "ISSUE_TEMPLATE.md"
105+
#label to tag the issue with if its missing information
106+
issue_label: needs more info
107+
#amount of sections from the template that can be missing information for the issue to still be considered complete
108+
acceptable_missing_info: 1
109+
steps:
110+
#Checkout the repo
111+
- uses: actions/checkout@v4
112+
113+
#Removes conflicting characters before using the issue content as a script parameter
114+
- name: Remove conflicting chars
115+
env:
116+
ISSUE_BODY: ${{github.event.issue.body}}
117+
uses: frabert/[email protected]
118+
id: remove_quotations
119+
with:
120+
pattern: "\""
121+
string: ${{env.ISSUE_BODY}}
122+
replace-with: '-'
123+
flags: g
124+
125+
#Checks for missing information inside the issue content
126+
- name: Check Information
127+
id: check-info
128+
env:
129+
ISSUE_BODY: ${{ steps.remove_quotations.outputs.replaced }}
130+
run: |
131+
echo "analysis_response=$(pwsh .\\.github\\scripts\\issue_analyzer.ps1 "${{ env.template }}" "${{ env.acceptable_missing_info }}" )" >> $GITHUB_ENV
132+
133+
#Closes the issue if the analysis response is "Empty"
134+
- name: Close issue
135+
if: env.analysis_response == 'Empty'
136+
run: |
137+
curl -v -u admin:${{ secrets.GITHUB_TOKEN }} -d '{"body": "${{env.comment_intro}} ${{env.close_issue_comment}} ${{env.info_needed}}"}' ${{ github.event.issue.url }}/comments
138+
curl -v -u admin:${{ secrets.GITHUB_TOKEN }} -X PATCH -d '{"state": "closed"}' ${{ github.event.issue.url }}
139+
140+
#Adds the "needs more info" label if needed
141+
- name: Label and comment issue
142+
if: ((env.analysis_response != 'Valid') && (env.analysis_response != 'Empty') && (github.event.action == 'opened'))
143+
run: |
144+
curl -v -u admin:${{ secrets.GITHUB_TOKEN }} -d '{"labels": ["${{env.issue_label}}"]}' ${{ github.event.issue.url }}/labels
145+
curl -v -u admin:${{ secrets.GITHUB_TOKEN }} -d '{"body": "${{env.comment_intro}} ${{env.needs_more_info_comment}} ${{env.specific_info}} ${{env.analysis_response}}.\n\n${{env.info_needed}}"}' ${{ github.event.issue.url }}/comments
146+
147+
#Removes the "needs more info" label if the issue has the missing information
148+
- name: Unlabel updated issue
149+
if: env.analysis_response == 'Valid' && github.event.action == 'edited'
150+
run: |
151+
echo urldecode ${{env.issue_label}}
152+
curl -v -u admin:${{ secrets.GITHUB_TOKEN }} -X DELETE ${{ github.event.issue.url }}/labels/$(echo -ne "${{env.issue_label}}" | xxd -plain | tr -d '\n' | sed 's/\(..\)/%\1/g')
153+
154+
#Adds greetings message
155+
- name: Greetings
156+
if: env.analysis_response == 'Valid' && github.event.action == 'opened'
157+
run: |
158+
curl -v -u admin:${{ secrets.GITHUB_TOKEN }} -d '{"body": "${{env.greetings_comment}}"}' ${{ github.event.issue.url }}/comments
159+
160+

0 commit comments

Comments
 (0)