Skip to content
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

feat(secret): Add built-in secrets rules for Private Packagist #7826

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

nicwortel
Copy link
Contributor

@nicwortel nicwortel commented Oct 30, 2024

Description

Private Packagist is a package repository for Composer, the dependency manager for PHP. It allows businesses to publish private packages for consumption within their organization, mirror open-source packages, receive vulnerability alerts, etc.

Private Packagist generates user and organization tokens for authentication.

All tokens are generated with a prefix and a checksum to help with automated secret scanning.
See https://packagist.com/docs/composer-authentication#token-format.

This pull requests adds built-in rules for the tokens generated by Private Packagist so Trivy can detect them.

Related issues

None.

If you think your change is trivial enough, you can skip the issue and instead add justification and explanation in the PR description.

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

Category: CategoryPrivatePackagist,
Title: "Private Packagist user token",
Severity: "HIGH",
// https://packagist.com/docs/composer-authentication#token-format
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see any other comments referring to the documentation of the token format, but I thought it would be useful.
Let me know if I should remove it.

@nicwortel
Copy link
Contributor Author

The Private Packagist token actually contains a checksum which can be calculated based on the prefix and random string, but as far as I'm aware Trivy doesn't support this, so I haven't implemented such a check.

ID: "private-packagist-user-token",
Category: CategoryPrivatePackagist,
Title: "Private Packagist user token",
Severity: "HIGH",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find any documentation on how to choose a severity level for secrets.
How should I determine the level?

Copy link
Contributor

Choose a reason for hiding this comment

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

it's hard to say what seriousness to choose for a particular secret.
Unfortunately, there is no documentation or rules for this
but I think HIGH is perfect for this secret

Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

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

hello @nicwortel
Thanks for contribution!

left small comments, take a look, please

Keywords: []string{"packagist_uut_"},
},
{
ID: "private-packagist-organization-token",
Copy link
Contributor

Choose a reason for hiding this comment

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

Tokens differ only in prefixes.
what if we combine them into one secret (private-packagist-token)?

Title: "Private Packagist user token",
Severity: "HIGH",
// https://packagist.com/docs/composer-authentication#token-format
Regex: MustCompile(`packagist_uut_(?i)[a-z0-9]{68}`),
Copy link
Contributor

Choose a reason for hiding this comment

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

a prefix, a 60 hexadecimal character long random part, and an eight hexadecimal character long checksum

IIUC we can use a-f0-9.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, not sure why I didn't think of that myself 🙈

@@ -0,0 +1,3 @@
ORG_READ_TOKEN=packagist_ort_6675e11a686c692f3f2e3b6ce528c3d122d22d912ea69a20713cdf51714ba710ad74
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
can you add a capital letter to one of the lines?

@nicwortel nicwortel force-pushed the secret-rules-packagist branch 2 times, most recently from 9038224 to 5920049 Compare November 17, 2024 13:39
Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

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

@nicwortel Thanks for your contribution!

LGTM

cc. @knqyf263

@DmitriyLewen
Copy link
Contributor

@nicwortel I don't have access to edit this PR
can you rebase it?

Private Packagist (Packagist.com) is a package repository for Composer,
the dependency manager for PHP.
Private Packagist generates user and organization tokens for
authentication.

All tokens are generated with a prefix and a checksum to help with
automated secret scanning.
See https://packagist.com/docs/composer-authentication#token-format.
@nicwortel nicwortel force-pushed the secret-rules-packagist branch from 5920049 to 6b30b8f Compare November 18, 2024 19:00
@nicwortel
Copy link
Contributor Author

@nicwortel I don't have access to edit this PR
can you rebase it?

Done 👍

@knqyf263 knqyf263 enabled auto-merge November 19, 2024 04:57
@knqyf263 knqyf263 added this pull request to the merge queue Nov 19, 2024
Merged via the queue into aquasecurity:main with commit 132d9df Nov 19, 2024
12 checks passed
@nicwortel nicwortel deleted the secret-rules-packagist branch November 19, 2024 16:19
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