Semgrep: Provide a unified mechanism to detect import of a specific package

Created on 25 Feb 2020  路  8Comments  路  Source: returntocorp/semgrep

Is your feature request related to a problem? Please describe.
I'm trying to ban use of a particular package in Python. Currently, I need to AND OR together a large number of patterns in order to detect all the various Python import styles.

Describe the solution you'd like
I'd like import package.name to fire on all variants of package import.

Here's an example:

I want to detect use of the deprecated Django package django.forms.extras. To do this, I have a (potentially nonexhaustive) pattern set:

patterns: - pattern-either: - pattern: from django.forms import extras - pattern: from django.forms.extras import $X - pattern: from django.forms import extras as $Y - pattern: from django.forms.extras import $X as $Y - pattern: import django.forms.extras - pattern: import django.forms.extras.$X - pattern: import django.forms.extras as $Y - pattern: import django.forms.extras.$X as $Y

I'd like to instead just write:
pattern: import django.forms.extras

enhancement external-user good first issue javascript python medium

Most helpful comment

Thx @inkz I've created a separate issue.
This issue should be fixed now. You can now use simple import pattern (e.g., import "foo.bar")
and it will match more complex form of imports.
This seems to work for Python, Js, and Go at least. I did not test for the other languages (e.g., Java, C) but the matching is done on the generic AST so this could work for free for those languages.

All 8 comments

Speaking from experience with Dlint, this is a pain.

For completeness sake, even though it's uncommon and likely even harder to detect, there's also:

from django.forms import *
...
extras.attribute(...)

Sounds like a good idea.

Speaking from experience with Dlint, this is a pain.

For completeness sake, even though it's uncommon and likely even harder to detect, there's also:

from django.forms import *
...
extras.attribute(...)

Good catch

It would be awesome to have this feature implemented for Javascript also :pray:
as it works pretty the same
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import

It would be awesome to have this feature implemented for Javascript also
as it works pretty the same
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import

Actually it's a bit different in Python because the original code still needs to reference the full path.
You do 'import foo.bar'
and in the code you have to still write 'foo.bar.z()'
which means with sgrep we can allow to write a pattern like 'foo.bar.z()' and still match code where
someone aliased the foo module, or the foo.bar module, or the foo.bar.z entity.

For Javascript what do you want @inkz ? what kind of equivalence you want to support.
Given a specific sgrep pattern, like foo.bar(...), what do you want this to match? Which form of aliasing you want to support?
I can imagine that if you did import {bar as w} from "/complex/path" then we may want a sgrep pattern like 'bar(...)' to also match in this case 'w(1)' because bar was aliased to w, but that means we will also match other code like 'import {bar} from "/anothercompletelydifferentmodule"

Maybe for JS you'll have to have another pattern: where you specify the specific module you want and we just handle the entity aliasing.

What would be great if have many sgrep patterns for JS and concrete code JS that we want to be matched against. Then I can implement it.

This introduces some regressions. You can not anymore match specific import constructions.

For example:

  • /home/pad/github/sgrep/sgrep/_build/default/bin/main_sgrep.exe -lang python -e from foo import * tests/python/
    /home/pad/github/sgrep/sgrep/tests/python/import_negatives.py:2
    import foo.bar.baz
    /home/pad/github/sgrep/sgrep/tests/python/import_negatives.py:4
    from foo.bar import baz
    /home/pad/github/sgrep/sgrep/tests/python/import_negatives.py:6
    from foo.bar import baz as b

even though we want to match only concrete code where someone use the bad 'import *' construct.

@aryx
Took some time to rethink your comment and try different code examples.

I agree with you, it would be way more practical to specify the module.
I broke down possible module import examples to this list:

/**/
let foo = require('./foo');
foo.something('yo');

/**/
let fooSome = require('./foo').something;
fooSome('yo1');

/**/
import * as foo1 from './foo';
foo1.something('yo2')

/**/
import {something} from './foo';
something('yo3')

/**/
import foofoo from "./foo";
foofoo.something('yo4')

/**/
import { something as smth } from "./foo";
smth('yo5')

/**/
import("./foo").then(foo2 => {
  foo2.something('yo6');
})

/**/
(async function() {
  const foo3 = await import('./foo');
  foo3.something('yo7')
})()

and here is how the rule to catch em all would look like:

rules:
  - id: imports
    patterns:
        - pattern-either:
            - pattern: |
                import $X from "./foo";
                $X.something(...);
            - pattern: |
                $E = require("./foo");
                $E.something(...);
    message: "imports test"
    languages: [javascript]
    severity: ERROR

so the basic idea is to specify the exact module path

import $X from "./foo";

but have all the aliasing handled

$X.something(...);

Thx @inkz I've created a separate issue.
This issue should be fixed now. You can now use simple import pattern (e.g., import "foo.bar")
and it will match more complex form of imports.
This seems to work for Python, Js, and Go at least. I did not test for the other languages (e.g., Java, C) but the matching is done on the generic AST so this could work for free for those languages.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

DrewDennison picture DrewDennison  路  4Comments

Mandawi picture Mandawi  路  4Comments

dev-zero picture dev-zero  路  4Comments

ajinabraham picture ajinabraham  路  6Comments

msorens picture msorens  路  5Comments