Skip to content

fix(CommandInjection): only throw on usages of github.event.|inputs#5

Merged
6f6d6172 merged 6 commits into
Betterment:mainfrom
btrautmann:bt/command-injection-only-violate-inputs
Aug 12, 2025
Merged

fix(CommandInjection): only throw on usages of github.event.|inputs#5
6f6d6172 merged 6 commits into
Betterment:mainfrom
btrautmann:bt/command-injection-only-violate-inputs

Conversation

@btrautmann

@btrautmann btrautmann commented Aug 11, 2025

Copy link
Copy Markdown
Contributor

I ran into a case where a PR was failing claws due to using github.event_name which should be entirely safe. I down-scoped the CommandInjection rule to only violate on run usages of github.event\. (newly modified regex) and inputs (existing regex) and added a spec to ensure non-inputs checks don't violate.

Note: This would still cause violations on things like github.event.pull_request.number which aren't user-generated and should be safe, but since it's impossible(?) to enumerate all of the non-free-form/user-generated fields on the event object, this change at least gives us things like github.event_name. Also, you can still use env as a way to safely use these properties.

@btrautmann btrautmann changed the title fix(CommandInjection): only throw on usages of github.event.inputs|inputs fix(CommandInjection): only throw on usages of github.event.|inputs Aug 11, 2025
Comment thread lib/claws/rule/command_injection.rb Outdated
btrautmann and others added 2 commits August 12, 2025 14:37
Co-authored-by: Omar A. <47008591+6f6d6172@users.noreply.github.com>
@6f6d6172 6f6d6172 requested a review from Copilot August 12, 2025 18:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the CommandInjection rule to be more precise in detecting potential security vulnerabilities by only flagging usages of github.event. and inputs. properties (with dots) rather than any reference to github.event or inputs.

  • Updated the regex pattern to require a dot after github.event and inputs to avoid false positives
  • Removed the requirement for spaces around the expression braces to catch more injection patterns
  • Added test coverage for both the new detection pattern and the exclusion of safe properties like github.event_name

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/claws/rule/command_injection.rb Updated regex pattern to only match github.event. and inputs. with dots, removing space requirements
spec/claws/rule/command_injection_spec.rb Added tests for expressions without spaces and verification that github.event_name doesn't trigger violations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread lib/claws/rule/command_injection.rb Outdated
btrautmann and others added 2 commits August 12, 2025 14:42
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

@6f6d6172 6f6d6172 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

platform lgtm
domain lgtm

@6f6d6172 6f6d6172 merged commit d187f17 into Betterment:main Aug 12, 2025
5 checks passed
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.

3 participants