-
Notifications
You must be signed in to change notification settings - Fork 0
fix: skill-guardrail #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8d2180b
080c8a7
424ceea
7484121
64648d4
ac0e82b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| github: Nadav011 | ||
| custom: ["https://mcpize.com/mcp/rtl-fixer"] |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,28 @@ | ||||||||||||||
| name: Security Scan | ||||||||||||||
|
|
||||||||||||||
| on: | ||||||||||||||
| push: | ||||||||||||||
| branches: [main] | ||||||||||||||
| pull_request: | ||||||||||||||
| branches: [main] | ||||||||||||||
|
|
||||||||||||||
| concurrency: | ||||||||||||||
| group: security-${{ github.ref }} | ||||||||||||||
| cancel-in-progress: true | ||||||||||||||
|
|
||||||||||||||
| jobs: | ||||||||||||||
| semgrep: | ||||||||||||||
| runs-on: [self-hosted, linux, x64, pop-os] | ||||||||||||||
| steps: | ||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||
| - name: Semgrep SAST | ||||||||||||||
| run: | | ||||||||||||||
| pip3 install --user semgrep 2>/dev/null || true | ||||||||||||||
| semgrep scan . --config=auto --error --severity ERROR | ||||||||||||||
|
|
||||||||||||||
| trivy: | ||||||||||||||
| runs-on: [self-hosted, linux, x64, pop-os] | ||||||||||||||
| steps: | ||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||
|
||||||||||||||
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| - name: Install Trivy | |
| run: | | |
| TRIVY_VERSION=0.51.2 | |
| curl -sfL https://raw.githubusercontent.com/aquasecurity/trivy/main/contrib/install.sh | sudo sh -s -- -b /usr/local/bin "v${TRIVY_VERSION}" |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,174 @@ | ||||||||||||||||||||||||||||||||||||||
| name: RTL Check | ||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Adding Useful? React with 👍 / 👎. |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||
| pull_request: | ||||||||||||||||||||||||||||||||||||||
| paths: | ||||||||||||||||||||||||||||||||||||||
| - "**/*.tsx" | ||||||||||||||||||||||||||||||||||||||
| - "**/*.jsx" | ||||||||||||||||||||||||||||||||||||||
| - "**/*.css" | ||||||||||||||||||||||||||||||||||||||
| - "**/*.vue" | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||
| rtl-lint: | ||||||||||||||||||||||||||||||||||||||
| name: RTL Logical Properties Check | ||||||||||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
| contents: read | |
| contents: read | |
| issues: write |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include left/right utility classes in violation regex
The regex does not include left-* or right-*, even though the check description says those classes are enforced. As a result, physical positioning utilities can be introduced without being flagged, creating false negatives for one of the directional patterns this workflow is intended to block.
Useful? React with 👍 / 👎.
Copilot
AI
Apr 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says left-{n} / right-{n} are checked, but the PATTERN does not include them. Either add \bleft- / \bright- (and handle the documented exception for left-0 right-0), or remove the claim from the comments so behavior matches documentation.
Copilot
AI
Apr 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scan regex misses common variants, so the workflow will silently pass on violations (e.g., rounded-tl-md, rounded-tr-lg, border-l without a trailing dash, negative classes like -ml-4, and ml-1/2 / other non-alnum modifiers). Update the pattern to cover these forms (or switch to a safer tokenization approach) so the check actually enforces the documented policy.
| # ml-{n} mr-{n} pl-{n} pr-{n} — physical margin/padding | |
| # text-left text-right — physical text alignment | |
| # border-l- border-r- — physical border sides | |
| # rounded-tl rounded-tr — physical border radius (corners) | |
| # rounded-bl rounded-br | |
| # float-left float-right — physical float | |
| # left-{n} right-{n} — physical inset positioning (when directional) | |
| PATTERN='\bml-[0-9a-z\[]|\bmr-[0-9a-z\[]|\bpl-[0-9a-z\[]|\bpr-[0-9a-z\[]|text-left\b|text-right\b|border-l-|border-r-|\brounded-tl\b|\brounded-tr\b|\brounded-bl\b|\brounded-br\b|float-left\b|float-right\b' | |
| # ml-* mr-* pl-* pr-* — physical margin/padding (including negatives, fractions, arbitrary values) | |
| # text-left text-right — physical text alignment | |
| # border-l border-r — physical border sides (bare or with modifiers) | |
| # rounded-tl rounded-tr — physical border radius (corners, bare or with modifiers) | |
| # rounded-bl rounded-br | |
| # float-left float-right — physical float | |
| # left-* right-* — physical inset positioning (when directional) | |
| PATTERN="(^|[^[:alnum:]_/-])(-?(ml|mr|pl|pr|left|right)-[^[:space:]\"'\`<>{}]+|text-(left|right)([^[:alnum:]_-]|$)|border-(l|r)(-[^[:space:]\"'\`<>{}]+)?([^[:alnum:]_-]|$)|rounded-(tl|tr|bl|br)(-[^[:space:]\"'\`<>{}]+)?([^[:alnum:]_-]|$)|float-(left|right)([^[:alnum:]_-]|$))" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scan repository files instead of hardcoded app folders
The RTL scan command only searches src/ components/ pages/ app/ styles/ and suppresses missing-path errors, so projects that keep UI code elsewhere (for example packages/*, apps/*, or lib/*) will report zero violations and incorrectly pass CI. This contradicts the step comment that it scans PR changes with a fallback, and it allows physical-direction regressions to merge unnoticed.
Useful? React with 👍 / 👎.
Copilot
AI
Apr 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scan is limited to src/ components/ pages/ app/ styles/, which can miss violations in other directories (and doesn’t implement the stated “search only changed files in the PR”). Consider scanning the whole repo (excluding vendor/build dirs) or deriving the changed file list from the PR and scanning only those paths.
Copilot
AI
Apr 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grep -v "rtl-ok" will exclude any line containing that substring (including string literals), not just suppression comments. If suppression is meant to be // rtl-ok, prefer a tighter filter (e.g., match comment patterns) and drop the redundant second grep once the filter is precise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pip3 install --user semgrepdoes not guaranteesemgrepis on PATH (user installs often land in~/.local/bin). On a fresh runner this can makesemgrep scanfail even though the install succeeded. Prefer installing in a way that ensures availability (e.g., add the user bin dir to PATH, usepipx, or invoke viapython -m semgrepif supported).