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(with-incident-details): Ensure the token has the correct scope #1015

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

salome-voltz
Copy link
Collaborator

Context

ggshield 1.32.0 can now return the details of an incident when a scan is run with the --with-incident-details option. For this feature to work, the token used to authenticate with the API must have the 'incidents:read' scope. This is currently not documented and if the token does not have the required scope, the flag will be silently ignored.

What has been done

Ensure the token has the required scope before running a command with --with-incident-details flag. This MR goes with GitGuardian/py-gitguardian#126

PR check list

  • As much as possible, the changes include tests (unit and/or functional)
  • If the changes affect the end user (new feature, behavior change, bug fix) then the PR has a changelog entry (see doc/dev/getting-started.md). If the changes do not affect the end user, then the skip-changelog label has been added to the PR.

@salome-voltz salome-voltz requested a review from a team as a code owner November 20, 2024 15:01
Copy link

codecov bot commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.04%. Comparing base (b82d7ca) to head (6462217).

Files with missing lines Patch % Lines
ggshield/verticals/secret/secret_scanner.py 90.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1015   +/-   ##
=======================================
  Coverage   92.04%   92.04%           
=======================================
  Files         181      181           
  Lines        7704     7717   +13     
=======================================
+ Hits         7091     7103   +12     
- Misses        613      614    +1     
Flag Coverage Δ
unittests 92.04% <94.11%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@salome-voltz salome-voltz force-pushed the salomevoltz/scrt-4913-ggshield-with-incident-details-does-not-tell-about-required branch 4 times, most recently from 265403f to b789dd7 Compare November 20, 2024 16:40
Copy link

@sevbch sevbch left a comment

Choose a reason for hiding this comment

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

Seems alright with me! 🙂 (though it's my very first review here)

(Don't forget to revert pdm update once GitGuardian/py-gitguardian#126 is released)

ggshield/cmd/secret/scan/secret_scan_common_options.py Outdated Show resolved Hide resolved
ggshield/core/errors.py Outdated Show resolved Hide resolved
@salome-voltz salome-voltz self-assigned this Nov 22, 2024
@salome-voltz salome-voltz force-pushed the salomevoltz/scrt-4913-ggshield-with-incident-details-does-not-tell-about-required branch from b789dd7 to 6ac5fc2 Compare November 22, 2024 14:18
@salome-voltz salome-voltz force-pushed the salomevoltz/scrt-4913-ggshield-with-incident-details-does-not-tell-about-required branch from 6ac5fc2 to 6462217 Compare November 22, 2024 14:30
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