Skip to content

Commit

Permalink
fix(with-incident-details): Ensure the token has the correct scope 'i…
Browse files Browse the repository at this point in the history
…ncident:read'
  • Loading branch information
salome-voltz committed Nov 25, 2024
1 parent b82d7ca commit 14d2c3d
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<!--
A new scriv changelog fragment.
Uncomment the section that is right (remove the HTML comment wrapper).
-->

<!--
### Removed
- A bullet item for the Removed category.
-->
<!--
### Added
- A bullet item for the Added category.
-->
<!--
### Changed
- A bullet item for the Changed category.
-->
<!--
### Deprecated
- A bullet item for the Deprecated category.
-->

### Fixed

- When `ggshield secret scan` is called with `--with-incident-details` and the token does not have the required scopes, the command fails and an error message is printed.

<!--
### Security
- A bullet item for the Security category.
-->
3 changes: 2 additions & 1 deletion ggshield/cmd/secret/scan/secret_scan_common_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def _banlist_detectors_callback(
metavar="DETECTOR",
)


_with_incident_details_option = click.option(
"--with-incident-details",
is_flag=True,
Expand All @@ -134,7 +135,7 @@ def _banlist_detectors_callback(
# If the option is placed early in the command line, the value may be overridden
# later on with False if no default is defined.
default=None,
help="Display full details about the dashboard incident if one is found (JSON and SARIF formats only).",
help="""Display full details about the dashboard incident if one is found (JSON and SARIF formats only). Requires the 'incidents:read' scope.""", # noqa
callback=create_config_callback("secret", "with_incident_details"),
)

Expand Down
18 changes: 16 additions & 2 deletions ggshield/core/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
import platform
import traceback
from enum import IntEnum
from typing import Any, Dict
from typing import Any, Dict, List

import click
from marshmallow import ValidationError
from pygitguardian.models import Detail
from pygitguardian.models import Detail, TokenScope

from ggshield.core.text_utils import pluralize
from ggshield.utils.git_shell import GitError, InvalidGitRefError


Expand Down Expand Up @@ -66,6 +67,19 @@ def __init__(self, message: str):
super().__init__(ExitCode.UNEXPECTED_ERROR, message)


class MissingScopesError(_ExitError):
"""
Token does not have the required scope
"""

def __init__(self, token_scopes: List[TokenScope]):
scopes_list = ", ".join([scope.value for scope in token_scopes])
super().__init__(
ExitCode.UNEXPECTED_ERROR,
f"Token is missing the required {pluralize('scope', len(token_scopes))} {scopes_list} to perform this operation.", # noqa
)


class AuthError(_ExitError):
"""
Base exception for Auth-related configuration error
Expand Down
14 changes: 12 additions & 2 deletions ggshield/verticals/secret/secret_scanner.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
from typing import Dict, Iterable, List, Optional, Union

from pygitguardian import GGClient
from pygitguardian.models import Detail, MultiScanResult
from pygitguardian.models import APITokensResponse, Detail, MultiScanResult, TokenScope

from ggshield.core import ui
from ggshield.core.cache import Cache
from ggshield.core.client import check_client_api_key
from ggshield.core.config.user_config import SecretConfig
from ggshield.core.constants import MAX_WORKERS
from ggshield.core.errors import handle_api_error
from ggshield.core.errors import MissingScopesError, UnexpectedError, handle_api_error
from ggshield.core.filter import (
remove_ignored_from_result,
remove_results_from_ignore_detectors,
Expand Down Expand Up @@ -63,6 +63,16 @@ def __init__(
self.headers = scan_context.get_http_headers()
self.command_id = scan_context.command_id

if secret_config.with_incident_details:
response = self.client.api_tokens()

if not isinstance(response, (Detail, APITokensResponse)):
raise UnexpectedError("Unexpected api_tokens response")

Check warning on line 70 in ggshield/verticals/secret/secret_scanner.py

View check run for this annotation

Codecov / codecov/patch

ggshield/verticals/secret/secret_scanner.py#L70

Added line #L70 was not covered by tests
elif isinstance(response, Detail):
raise UnexpectedError(response.detail)
if TokenScope.INCIDENTS_READ not in response.scopes:
raise MissingScopesError([TokenScope.INCIDENTS_READ])

def scan(
self,
files: Iterable[Scannable],
Expand Down
11 changes: 5 additions & 6 deletions pdm.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ dependencies = [
"marshmallow~=3.18.0",
"marshmallow-dataclass~=8.5.8",
"oauthlib~=3.2.1",
"pygitguardian~=1.17.0",
"pygitguardian @ git+https://github.com/GitGuardian/py-gitguardian.git@41d034799683d7b6c1fd4e7131f9a740c4d9851d",
"pyjwt~=2.6.0",
"python-dotenv~=0.21.0",
"pyyaml~=6.0.1",
Expand Down
15 changes: 15 additions & 0 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,21 @@ def is_macos():
}
)

API_TOKENS_RESPONSE_SCAN_SCOPES = {
"id": "5ddaad0c-5a0c-4674-beb5-1cd198d13360",
"name": "myTokenName",
"workspace_id": 42,
"type": "personal_access_token",
"status": "revoked",
"created_at": "2023-05-20T12:40:55.662949Z",
"last_used_at": "2023-05-24T12:40:55.662949Z",
"expire_at": None,
"revoked_at": "2023-05-27T12:40:55.662949Z",
"member_id": 22015,
"creator_id": 22015,
"scopes": ["scan"],
}

# This long token is a test token, always reported as an uncheckable secret
GG_TEST_TOKEN = (
"8a784aab7090f6a4ba3b9f7a6594e2e727007a26590b58ed314e4b9ed4536479sRZlRup3xvtMVfiHWA"
Expand Down
43 changes: 41 additions & 2 deletions tests/unit/verticals/secret/test_secret_scanner.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,15 @@

import click
import pytest
from pygitguardian.models import Detail
from pygitguardian.models import APITokensResponse, Detail

from ggshield.core.config.user_config import SecretConfig
from ggshield.core.errors import ExitCode, QuotaLimitReachedError
from ggshield.core.errors import (
ExitCode,
MissingScopesError,
QuotaLimitReachedError,
UnexpectedError,
)
from ggshield.core.scan import (
Commit,
DecodeError,
Expand All @@ -24,6 +29,7 @@
_NO_SECRET_PATCH,
_ONE_LINE_AND_MULTILINE_PATCH,
_SIMPLE_SECRET_TOKEN,
API_TOKENS_RESPONSE_SCAN_SCOPES,
GG_TEST_TOKEN,
UNCHECKED_SECRET_PATCH,
my_vcr,
Expand Down Expand Up @@ -241,3 +247,36 @@ def test_scan_merge_commit(client, cache):
matches = {m.match_type: m.match for m in scan.policy_breaks[0].matches}
assert matches["username"] == "owly"
assert matches["password"] == _SIMPLE_SECRET_TOKEN


@pytest.mark.parametrize(
"api_response, expected_exception, message",
[
(
Detail(detail="Unexpected response"),
UnexpectedError,
"Unexpected response",
),
(
APITokensResponse.from_dict(API_TOKENS_RESPONSE_SCAN_SCOPES),
MissingScopesError,
"Token is missing the required scope incidents:read to perform this operation.",
),
],
)
def test_with_incident_details_error(
monkeypatch, client, cache, api_response, expected_exception, message
):
monkeypatch.setattr(client, "api_tokens", Mock(return_value=api_response))
with pytest.raises(expected_exception) as exc_info:
SecretScanner(
client=client,
cache=cache,
scan_context=ScanContext(
scan_mode=ScanMode.PATH,
command_path="external",
),
check_api_key=False,
secret_config=SecretConfig(with_incident_details=True),
)
assert message in str(exc_info.value)

0 comments on commit 14d2c3d

Please sign in to comment.