Take explicit permission into account for privilege calculation

ql/lib/codeql/actions/ast/internal/Ast.qll

@@ -848,11 +848,23 @@ class JobImpl extends AstNodeImpl, TJobNode {
     this.getPermissions().getAPermission().matches("%write")
   }
 
+  private predicate hasExplicitReadPermission() {
+    // the job has not an explicit write permission
+    exists(this.getPermissions().getAPermission()) and
+    not this.getPermissions().getAPermission().matches("%write")
+  }
+
   private predicate hasImplicitWritePermission() {
     // the job has an explicit write permission
     this.getEnclosingWorkflow().getPermissions().getAPermission().matches("%write")
   }
 
+  private predicate hasImplicitReadPermission() {
+    // the job has not an explicit write permission
+    exists(this.getEnclosingWorkflow().getPermissions().getAPermission()) and
+    not this.getEnclosingWorkflow().getPermissions().getAPermission().matches("%write")
+  }
+
   private predicate hasRuntimeData() {
     exists(string path, string trigger, string name, string secrets_source, string perms |
       workflowDataModel(path, trigger, name, secrets_source, perms, _) and
@@ -892,8 +904,7 @@ class JobImpl extends AstNodeImpl, TJobNode {
 
   /** Holds if the action is privileged and externally triggerable. */
   predicate isPrivilegedExternallyTriggerable() {
-    exists(EventImpl e |
-      this.getATriggerEvent() = e and
+    exists(EventImpl e | this.getATriggerEvent() = e |
       // job is triggereable by an external user
       e.isExternallyTriggerable() and
       // no matter if `pull_request` is granted write permissions or access to secrets
@@ -903,9 +914,18 @@ class JobImpl extends AstNodeImpl, TJobNode {
         // job is privileged (write access or access to secrets)
         this.isPrivileged()
         or
-        // the trigger event is __normally__ privileged and we have no runtime data to prove otherwise
+        // the trigger event is __normally__ privileged
+        e.isPrivileged() and
+        // and we have no runtime data to prove otherwise
         not this.hasRuntimeData() and
-        e.isPrivileged()
+        // and the job is not explicitly non-privileged
+        not (
+          (
+            this.hasExplicitReadPermission() or
+            this.hasImplicitReadPermission()
+          ) and
+          not this.hasExplicitSecretAccess()
+        )
       )
     )
   }

ql/test/query-tests/Security/CWE-349/.github/workflows/test20.yml

@@ -0,0 +1,46 @@
+name: Publish
+
+on:
+  push:
+    branches:
+      - main
+  pull_request_target:
+  workflow_dispatch:
+  workflow_call:
+
+jobs:
+  build-and-upload:
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+    steps:
+
+    - name: Checkout PR
+      if: ${{ github.event_name == 'pull_request_target' }}
+      uses: actions/checkout@v3
+      with:
+        ref: ${{ github.event.pull_request.head.ref }}
+        repository: ${{ github.event.pull_request.head.repo.full_name }}
+
+    - name: Checkout
+      if: ${{ github.event_name != 'pull_request_target' }}
+      uses: actions/checkout@v3
+      with:
+        ref: main
+    
+    - name: Setup Pages
+      uses: actions/configure-pages@v1
+    - name: Use Node.js
+      uses: actions/setup-node@v3
+      with:
+        node-version: 18
+        cache: npm
+    - name: Update npm to latest
+      run: npm i --prefer-online --no-fund --no-audit -g npm@latest
+    - run: npm -v
+    - run: npm i --ignore-scripts --no-audit --no-fund --package-lock
+    - run: npm run build -w www
+    - name: Upload artifact
+      uses: actions/upload-pages-artifact@v1
+      with:
+        path: './workspaces/www/build'

ql/test/query-tests/Security/CWE-829/.github/workflows/test4.yml

@@ -0,0 +1,46 @@
+name: Publish
+
+on:
+  push:
+    branches:
+      - main
+  pull_request_target:
+  workflow_dispatch:
+  workflow_call:
+
+jobs:
+  build-and-upload:
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+    steps:
+
+    - name: Checkout PR
+      if: ${{ github.event_name == 'pull_request_target' }}
+      uses: actions/checkout@v3
+      with:
+        ref: ${{ github.event.pull_request.head.ref }}
+        repository: ${{ github.event.pull_request.head.repo.full_name }}
+
+    - name: Checkout
+      if: ${{ github.event_name != 'pull_request_target' }}
+      uses: actions/checkout@v3
+      with:
+        ref: main
+    
+    - name: Setup Pages
+      uses: actions/configure-pages@v1
+    - name: Use Node.js
+      uses: actions/setup-node@v3
+      with:
+        node-version: 18
+        cache: npm
+    - name: Update npm to latest
+      run: npm i --prefer-online --no-fund --no-audit -g npm@latest
+    - run: npm -v
+    - run: npm i --ignore-scripts --no-audit --no-fund --package-lock
+    - run: npm run build -w www
+    - name: Upload artifact
+      uses: actions/upload-pages-artifact@v1
+      with:
+        path: './workspaces/www/build'

ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected

@@ -4,6 +4,5 @@
 | .github/workflows/level0.yml:99:9:103:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
 | .github/workflows/level0.yml:125:9:129:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
 | .github/workflows/mend.yml:22:9:29:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
-| .github/workflows/test3.yml:28:9:33:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
 | .github/workflows/untrusted_checkout.yml:10:9:13:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
 | .github/workflows/untrusted_checkout.yml:13:9:16:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |

ql/test/query-tests/Security/CWE-829/UntrustedCheckoutMedium.expected

@@ -1,3 +1,5 @@
 | .github/workflows/dependabot1.yml:15:9:19:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
 | .github/workflows/dependabot1.yml:39:9:43:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
 | .github/workflows/priv_pull_request_checkout.yml:14:9:20:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
+| .github/workflows/test3.yml:28:9:33:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |
+| .github/workflows/test4.yml:18:7:25:4 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |