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

fix(detect-scan): support input for .secrets.baseline path #51

Merged
merged 3 commits into from
Jan 15, 2024
Merged

fix(detect-scan): support input for .secrets.baseline path #51

merged 3 commits into from
Jan 15, 2024

Conversation

SoaAlex
Copy link
Contributor

@SoaAlex SoaAlex commented Jan 12, 2024

Issue

When .secrets.baseline is provided, the file is only updated and not written to stdout.
Therefore, the pipe to baseline2rdf doesn't work as there is no JSON to parse.

Proposed solution

  • Add an input to specify a baseline path (usually .secrets.baseline).
  • If an input is provided, just update the current baseline file (rename to conventional name if different). If not, output the json from stdout to .secrets.baseline
  • Echo the content of .secrets.baseline and pipe it to baseline2rdf and reviewdog commands.

This was not tested, and it might contains errors, please don't hesitate to fix it or suggest any modification :)

Copy link
Member

@levonet levonet left a comment

Choose a reason for hiding this comment

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

This approach will create and store the artifact .secrets.baseline in the working directory. This is unexpected behavior for an optional feature.

I suggest moving the .secrets.baseline file to a temporary container directory /tmp/. And if stdout data needs access, then changing the path to the file as addition option.

@SoaAlex
Copy link
Contributor Author

SoaAlex commented Jan 15, 2024

This approach will create and store the artifact .secrets.baseline in the working directory. This is unexpected behavior for an optional feature.

I suggest moving the .secrets.baseline file to a temporary container directory /tmp/. And if stdout data needs access, then changing the path to the file as addition option.

Thanks for your suggestion.
I agree and have moved the .secrets.baseline to /tmp. I'm not sure this is all you suggested.

Copy link
Member

@levonet levonet left a comment

Choose a reason for hiding this comment

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

It seems that these changes will not change the usual behavior. Ping me if I can merge it.

@SoaAlex
Copy link
Contributor Author

SoaAlex commented Jan 15, 2024

It seems that these changes will not change the usual behavior. Ping me if I can merge it.

@levonet Thanks ! You can merge it

@levonet levonet merged commit f9d3255 into reviewdog:master Jan 15, 2024
4 checks passed
Copy link
Contributor

🚀 [bumpr] Bumped! New version:v0.15.0 Changes:v0.14.0...v0.15.0

@review-dog
Copy link
Member

Hi, @SoaAlex! We merged your PR to reviewdog! 🐶
Thank you for your contribution! ✨

We just invited you to join the @reviewdog organization on GitHub.
Accept the invite by visiting https://github.com/orgs/reviewdog/invitation.
By joining the team, you'll be a part of reviewdog community and can help the maintenance of reviewdog.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants