Skip to content

Commit e5aa4a9

Browse files
authored
Merge pull request #1132 from GitGuardian/gcasier/SCRT-5952/add_insecure_option
feat: add --insecure option and deprecate --allow-self-signed
2 parents f92b323 + 8983a6e commit e5aa4a9

File tree

13 files changed

+151
-16
lines changed

13 files changed

+151
-16
lines changed
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<!--
2+
A new scriv changelog fragment.
3+
4+
Uncomment the section that is right (remove the HTML comment wrapper).
5+
For top level release notes, leave all the headers commented out.
6+
-->
7+
8+
### Added
9+
10+
- Added `--insecure` CLI option and `insecure` configuration setting as clearer alternatives to `--allow-self-signed` and `allow_self_signed`. The new option explicitly communicates that SSL verification is completely disabled, making the connection vulnerable to man-in-the-middle attacks.
11+
- Added prominent warning messages when SSL verification is disabled (via either `--insecure` or `--allow-self-signed`), explaining the security risks and recommending the secure alternative of using the system certificate trust store (available with Python >= 3.10).
12+
13+
### Deprecated
14+
15+
- The `--allow-self-signed` CLI option and `allow_self_signed` configuration setting are now deprecated in favor of `--insecure` and `insecure`. Deprecation warnings are displayed when these options are used, guiding users to the clearer alternative. Both options remain functional for backward compatibility and will be maintained for an extended deprecation period before removal.
16+
17+
### Security
18+
19+
- Improved clarity around SSL verification settings. The `--allow-self-signed` option name was misleading as it suggests certificate validation is still performed, when in reality all SSL verification is disabled. The new `--insecure` option makes this behavior explicit. Both options remain functional for backward compatibility.
20+
21+
<!--
22+
### Changed
23+
24+
- A bullet item for the Changed category.
25+
26+
-->
27+
<!--
28+
### Removed
29+
30+
- A bullet item for the Removed category.
31+
32+
-->
33+
<!--
34+
### Deprecated
35+
36+
- A bullet item for the Deprecated category.
37+
38+
-->
39+
<!--
40+
### Fixed
41+
42+
- A bullet item for the Fixed category.
43+
44+
-->

ggshield/__main__.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ def cli(
8282
ctx: click.Context,
8383
*,
8484
allow_self_signed: Optional[bool],
85+
insecure: Optional[bool],
8586
config_path: Optional[Path],
8687
**kwargs: Any,
8788
) -> None:
@@ -97,11 +98,11 @@ def cli(
9798
elif user_config.verbose and ui.get_level() < ui.Level.VERBOSE:
9899
ensure_level(ui.Level.VERBOSE)
99100

100-
# Update allow_self_signed in the config
101+
# Update SSL verification settings in the config
101102
# TODO: this should be reworked: if a command which writes the config is called with
102-
# --allow-self-signed, the config will contain `allow_self_signed: true`.
103-
if allow_self_signed:
104-
user_config.allow_self_signed = allow_self_signed
103+
# --insecure, the config will contain `insecure: true`.
104+
if insecure or allow_self_signed:
105+
user_config.insecure = True
105106

106107
ctx_obj.config._dotenv_vars = load_dot_env()
107108

ggshield/cmd/auth/login.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ def token_login(config: Config, instance: Optional[str]) -> None:
192192
client = create_client(
193193
api_key=token,
194194
api_url=config.api_url,
195-
allow_self_signed=config.user_config.allow_self_signed,
195+
allow_self_signed=config.user_config.insecure,
196196
)
197197
try:
198198
response = client.get(endpoint="token")

ggshield/cmd/auth/logout.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ def revoke_token(config: Config, instance_url: str) -> None:
9696
client = create_client(
9797
token,
9898
dashboard_to_api_url(instance_url),
99-
allow_self_signed=config.user_config.allow_self_signed,
99+
allow_self_signed=config.user_config.insecure,
100100
)
101101
try:
102102
response = client.post(endpoint="token/revoke")

ggshield/cmd/utils/common_options.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,30 @@ def log_file_callback(
136136
)
137137

138138

139+
def allow_self_signed_callback(
140+
ctx: click.Context, param: click.Parameter, value: Optional[bool]
141+
) -> Optional[bool]:
142+
if value:
143+
ui.display_warning(
144+
"The --allow-self-signed option is deprecated. Use --insecure instead."
145+
)
146+
return create_config_callback("insecure")(ctx, param, value)
147+
148+
139149
_allow_self_signed_option = click.option(
140150
"--allow-self-signed",
141151
is_flag=True,
142152
default=None,
143-
help="Ignore ssl verification.",
144-
callback=create_config_callback("allow_self_signed"),
153+
help="Deprecated: use --insecure.",
154+
callback=allow_self_signed_callback,
155+
)
156+
157+
_insecure_option = click.option(
158+
"--insecure",
159+
is_flag=True,
160+
default=None,
161+
help="Skip all certificate verification checks. WARNING: this option makes the transfer insecure.",
162+
callback=create_config_callback("insecure"),
145163
)
146164

147165
_check_for_updates = click.option(
@@ -174,6 +192,7 @@ def decorator(cmd: AnyFunction) -> AnyFunction:
174192
_debug_option(cmd)
175193
_log_file_option(cmd)
176194
_allow_self_signed_option(cmd)
195+
_insecure_option(cmd)
177196
_check_for_updates(cmd)
178197
return cmd
179198

ggshield/core/client.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from pygitguardian.models import APITokensResponse, Detail, TokenScope
99
from requests import Session
1010

11+
from . import ui
1112
from .config import Config
1213
from .constants import DEFAULT_INSTANCE_URL
1314
from .errors import (
@@ -54,7 +55,7 @@ def create_client_from_config(config: Config) -> GGClient:
5455
return create_client(
5556
api_key,
5657
api_url,
57-
allow_self_signed=config.user_config.allow_self_signed,
58+
allow_self_signed=config.user_config.insecure,
5859
callbacks=callbacks,
5960
)
6061

@@ -88,6 +89,16 @@ def create_client(
8889
def create_session(allow_self_signed: bool = False) -> Session:
8990
session = Session()
9091
if allow_self_signed:
92+
ui.display_warning(
93+
"SSL verification is disabled. Your connection to the GitGuardian API is NOT encrypted "
94+
"and is vulnerable to man-in-the-middle attacks. Traffic, including API keys and scan results, "
95+
"can be intercepted and modified."
96+
)
97+
ui.display_warning(
98+
"To securely use self-signed certificates with Python >= 3.10, disable this option and "
99+
"install your certificate in your system's trust store. "
100+
"See: https://docs.gitguardian.com/ggshield-docs/configuration#support-for-self-signed-certificates"
101+
)
91102
urllib3.disable_warnings()
92103
session.verify = False
93104
return session

ggshield/core/config/user_config.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class UserConfig(FilteredConfig):
8787
instance: Optional[str] = None
8888
exit_zero: bool = False
8989
verbose: bool = False
90-
allow_self_signed: bool = False
90+
insecure: bool = False
9191
max_commits_for_hook: int = 50
9292
secret: SecretConfig = field(default_factory=SecretConfig)
9393
debug: bool = False
@@ -198,6 +198,7 @@ def _load_config_dict(
198198
if dash_keys:
199199
_warn_about_dash_keys(config_path, dash_keys)
200200
_fix_ignore_known_secrets(dct)
201+
_fix_allow_self_signed(dct, config_path)
201202
elif config_version == 1:
202203
deprecation_messages.append(
203204
f"{config_path} uses a deprecated configuration file format."
@@ -226,6 +227,16 @@ def _fix_ignore_known_secrets(data: Dict[str, Any]) -> None:
226227
secret_dct[_IGNORE_KNOWN_SECRETS_KEY] = value
227228

228229

230+
def _fix_allow_self_signed(data: Dict[str, Any], config_path: Path) -> None:
231+
"""Convert allow_self_signed to insecure and display a deprecation warning."""
232+
if insecure := data.pop("allow_self_signed", None):
233+
ui.display_warning(
234+
f"{config_path}: The 'allow_self_signed' option is deprecated. "
235+
"Use 'insecure: true' instead."
236+
)
237+
data["insecure"] = insecure
238+
239+
229240
def _warn_about_dash_keys(config_path: Path, dash_keys: Set[str]) -> None:
230241
for old_key in sorted(dash_keys):
231242
new_key = old_key.replace("-", "_")

ggshield/core/config/v1_config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def copy_if_set(dct: Dict[str, Any], dst_key: str, src_key: Optional[str] = None
6060
copy_if_set(dct, "instance")
6161
copy_if_set(dct, "exit_zero")
6262
copy_if_set(dct, "verbose")
63-
copy_if_set(dct, "allow_self_signed")
63+
copy_if_set(dct, "insecure", "allow_self_signed")
6464
copy_if_set(dct, "max_commits_for_hook")
6565

6666
return dct

ggshield/verticals/auth/oauth.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ def _claim_token(self, authorization_code: str) -> None:
231231
body=urlparse.urlencode(request_params),
232232
)
233233

234-
session = create_session(self.config.user_config.allow_self_signed)
234+
session = create_session(self.config.user_config.insecure)
235235
response = session.post(
236236
urljoin(self.api_url, "/v1/oauth/token"),
237237
request_body,
@@ -265,7 +265,7 @@ def _validate_access_token(self) -> Dict[str, Any]:
265265
response = create_client(
266266
self._access_token,
267267
self.api_url,
268-
allow_self_signed=self.config.user_config.allow_self_signed,
268+
allow_self_signed=self.config.user_config.insecure,
269269
).get(endpoint="token")
270270
if not response.ok:
271271
raise OAuthError("The created token is invalid.")

ggshield/verticals/hmsl/utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def get_token(config: Config) -> Optional[str]:
8282
client = create_client(
8383
api_url=config.saas_api_url,
8484
api_key=config.saas_api_key,
85-
allow_self_signed=config.user_config.allow_self_signed,
85+
allow_self_signed=config.user_config.insecure,
8686
)
8787
audience = config.hmsl_audience
8888
except (MissingTokenError, AuthExpiredError):

0 commit comments

Comments
 (0)