-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: prevent uploading PyPI tokens in common places #19994
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
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 |
|---|---|---|
|
|
@@ -16,9 +16,21 @@ | |
| # YARA rules directory | ||
| _RULES_DIR = Path(__file__).parent / "scanner_rules" | ||
|
|
||
| # Extensions to scan inside archives. Python source (.py) for source-level | ||
| # rules (e.g. pyarmor), and .pye for SourceDefender-encrypted files. | ||
| _SCAN_EXTENSIONS = {".py", ".pye"} | ||
| # Extensions to scan inside archives. | ||
| _SCAN_EXTENSIONS = { | ||
|
kam193 marked this conversation as resolved.
Outdated
|
||
| # Python source for source-level rules (e.g. pyarmor) | ||
| ".py", | ||
| # .pye for SourceDefender-encrypted files | ||
| ".pye", | ||
| # Different textual files for common places where PyPI tokens are accidentally left. | ||
| ".md", | ||
| ".rst", | ||
| ".env", | ||
|
Member
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. Nit: it might be good to match anything with
Author
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. Yes - see the previous comment. As a bit of context, the current list is based on where I saw tokens recently. |
||
| ".sh", | ||
| ".txt", | ||
| "METADATA", | ||
| "PKG-INFO", | ||
| } | ||
|
|
||
| # Max size of individual file to scan inside archive (5 MiB) | ||
| _SCAN_MAX_FILE_SIZE = 5 * 1024 * 1024 | ||
|
|
@@ -78,8 +90,10 @@ def iter_zip_members(zfp: zipfile.ZipFile) -> typing.Iterator[tuple[str, int, by | |
| for entry in zfp.infolist(): | ||
| if entry.is_dir(): | ||
| continue | ||
| ext = Path(entry.filename).suffix.lower() | ||
| if ext not in _SCAN_EXTENSIONS: | ||
| path = Path(entry.filename) | ||
| ext = path.suffix.lower() | ||
| # Names like "METADATA", ".env" have empty suffix | ||
| if ext not in _SCAN_EXTENSIONS and path.name not in _SCAN_EXTENSIONS: | ||
| continue | ||
| data = zfp.read(entry.filename) | ||
| yield entry.filename, len(data), data | ||
|
|
@@ -90,8 +104,10 @@ def iter_tar_members(tar: tarfile.TarFile) -> typing.Iterator[tuple[str, int, by | |
| for member in tar.getmembers(): | ||
| if not member.isfile(): | ||
| continue | ||
| ext = Path(member.name).suffix.lower() | ||
| if ext not in _SCAN_EXTENSIONS: | ||
| path = Path(member.name) | ||
| ext = path.suffix.lower() | ||
| # Names like "PKG-INFO", ".env" have empty suffix | ||
| if ext not in _SCAN_EXTENSIONS and path.name not in _SCAN_EXTENSIONS: | ||
| continue | ||
| f = tar.extractfile(member) | ||
| if f is None: # pragma: no cover | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| rule secrets_pypi_token | ||
| { | ||
| meta: | ||
| description = "Detects PyPI API tokens exposed in source code." | ||
|
Member
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. question: I think the other rules in the repo use spaces - which is more adopted by the community? whichever it is, it should be set it in the root |
||
| author = "Kamil Mankowski" | ||
| message = "We have detected a PyPI API token exposed in the uploaded file. Publishing it would allow anyone to perform actions on your behalf. For your own security, please revoke the token immediately." | ||
|
kam193 marked this conversation as resolved.
Outdated
|
||
|
|
||
| strings: | ||
| // Regex adapted from trufflehog's PyPI token detector | ||
| // Intentionally not derived from the official Token format definition to spare unnecessary matches. | ||
| // Pre-computed head ensures we match actual pypi.org tokens | ||
| // https://github.com/trufflesecurity/trufflehog/blob/main/pkg/detectors/pypi/pypi.go | ||
|
Member
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. great addition to using a reference - better to make it a permalink like https://github.com/trufflesecurity/trufflehog/blob/05a583290be8c8f79852eb8741f5042920b47d00/pkg/detectors/pypi/pypi.go#L28 |
||
| $pypi_token = /pypi-AgEIcHlwaS5vcmcCJ[a-zA-Z0-9-_]{150,157}/ | ||
|
|
||
| // TODO: look if there are test tokens in use we should exclude | ||
| // $test_token = "pypi-AgEIcHlwaS5vcmcCJxxx" | ||
|
Comment on lines
+15
to
+16
Member
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. I don't think there are any that I'd expect to be included in packages? I have found examples of the exact same regex included in some pacakges that do their own scanning/detection - some examples: I'd want to allow those through - how would this filter work for them? Probably good examples to add to the test cases. |
||
|
|
||
| condition: | ||
| $pypi_token | ||
| // If we want to allow some test-only tokens, we can use: | ||
| // and for all i in (1 .. #pypi_token) : ( | ||
| // not $test_token at @pypi_token[i] | ||
| // ) | ||
| } | ||
|
kam193 marked this conversation as resolved.
Outdated
|
||
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.
I can imagine a future factory-style generator to create tokens for testing, so this paves that future nicely.