Skip to content
Notifications
Clear all

Step-by-step: creating a custom rule for our internal API framework

5 Posts
5 Users
0 Reactions
0 Views
(@integration_jane_new)
Estimable Member
Joined: 4 months ago
Posts: 111
Topic starter   [#4557]

Having recently completed a comprehensive integration project that required securing our internal GraphQL-based API framework, I found myself needing to enforce several security and design patterns that out-of-the-box Semgrep rules couldn't address. Specifically, our framework uses custom decorators for permission checks and has a particular pattern for error propagation that we wanted to codify. This post details the step-by-step process I followed to create, test, and deploy a custom rule, which may serve as a template for others looking to enforce domain-specific patterns.

The first step involved identifying the precise code pattern we wanted to catch. In our case, we wanted to ensure that all resolver functions in our GraphQL modules were decorated with one of our custom `@permissions()` decorators. A missing decorator could lead to an unauthorized data leak. I began by examining the Abstract Syntax Tree (AST) of a correct example to understand its structure. Using the `semgrep --debug` command on a sample file was invaluable here.

```yaml
rules:
- id: internal-api-missing-permission-decorator
message: "Resolver function is missing the required @permissions() decorator."
severity: ERROR
patterns:
- pattern-either:
- pattern: |
@$MODULE.router.$RESOLVER(...)
def $FUNC(...):
...
- pattern: |
@$MODULE.$RESOLVER(...)
def $FUNC(...):
...
fix: |
@permissions("default_scope", "read")
languages: [python]
metadata:
category: "security"
framework: "internal-api"
```

However, the initial rule above was too simplistic. It flagged every decorated resolver, which was the inverse of what we needed. We needed to find resolvers *missing* the decorator. This required using a `pattern-not` clause to exclude functions that already had the correct decorator pattern. The key was to craft a pattern that matched any resolver-looking function but then subtract those with our specific `@permissions(...)` decorator. The final rule logic became:

1. Match a function definition that appears to be a resolver (by its decorator naming convention, e.g., `@router.query`).
2. But exclude any such function that already has a `@permissions` decorator applied (accounting for its potential position among other decorators).

The development and iterative testing were done using a dedicated test file with both positive cases (violations we want to catch) and negative cases (code that should pass). The Semgrep test functionality (`semgrep --test`) is crucial for ensuring rule accuracy and preventing regressions. After validation, the rule was integrated into our CI/CD pipeline by adding it to our centralized `.semgrep.yml` configuration, which now includes a mix of community and internal rules. The entire process, from AST exploration to pipeline deployment, took approximately half a day and has already identified several legacy endpoints lacking proper authorization checks.



   
Quote
(@mikeb22)
Active Member
Joined: 1 week ago
Posts: 5
 

That's a solid approach. The semgrep debug flag is a lifesaver for understanding the AST, especially with decorators. One thing that tripped me up early on was making sure my pattern matched both class methods and standalone functions. I ended up using a pattern like `function $FUNC(...) { ... }` and then a separate one for `$CLASS.$METHOD(...) { ... }` in the same rule.

Also, did you find a clean way to handle the different decorator argument styles? Sometimes our teams use `@permissions()` and sometimes just `@permissions`. I had to account for both.


Data is the new oil, but it's messy.


   
ReplyQuote
(@lizzieb)
Eminent Member
Joined: 1 week ago
Posts: 18
 

Oh, handling both decorator styles was tricky. I used a pattern like `@permissions(...)` and made the parentheses and their contents optional with a regex. So something like `@permissions(...)` where the ellipsis is the actual pattern for optional args.

Your point about class methods is good. I ran into that and my pattern missed them at first, which made the rule feel useless. I like your approach of having two separate patterns in the same rule. Did you find any performance hit when scanning large codebases with that setup?



   
ReplyQuote
(@michellet)
Active Member
Joined: 1 week ago
Posts: 11
 

The approach of starting with AST inspection using `semgrep --debug` is indeed the correct foundation. However, I'd caution against the implicit assumption in your message that a missing `@permissions()` decorator is the primary failure mode. In my work with similar frameworks, I've found the more insidious issue is the presence of a decorator with an incorrect or overly permissive argument, like `@permissions('public')` when it should be `@permissions('admin', 'auditor')`. A rule that only checks for the decorator's presence misses that critical logic flaw.

Your rule's message mentions a missing decorator, but your pattern fragment is incomplete. To be effective, you need a pattern that specifically targets resolver function definitions, which in most GraphQL implementations are async functions within specific module files, not all functions. You'll need to combine the decorator check with a `patterns` block that first identifies a resolver, perhaps by its common signature like `async def resolve_$FIELD(self, info, **kwargs)`, and then a `pattern-not` for the required decorator. Without that scope, you'll generate false positives on every helper function in the file.

Regarding the optional parentheses discussed later in the thread, a metavariable ellipsis like `@permissions(...)` handles both `@permissions` and `@permissions()` inherently, as the `...` matches zero or more arguments. No regex is required for that particular aspect.


Query first, ask questions later.


   
ReplyQuote
(@devops_dad_joke_v3)
Estimable Member
Joined: 3 months ago
Posts: 103
 

Right? It's like locking the front door but leaving the window open with a sign that says "admin only." The real bug is letting `@permissions('guest')` into prod.

You're spot on about scoping. Your pattern-not approach is the way. Just add a check for bad arguments too. Flag the decorator with a wildcard argument that *isn't* in your approved list. Catching both the missing and the malformed in one pass.

Sounds like you've been burned before. I bet you have a story about the `@permissions('all')` incident. We all do 😬


Deploy with love


   
ReplyQuote