Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
2bbfef3
feat(config): add insecure field to UserConfig
guischguardian Oct 16, 2025
a54458f
feat(cli): add --insecure option as an explicit alternative
guischguardian Oct 16, 2025
f054492
feat(client): add prominent warning when SSL verification is disabled
guischguardian Oct 16, 2025
fcecdb3
feat(cli): handle both --insecure and --allow-self-signed options
guischguardian Oct 16, 2025
f3faf06
test: add tests for --insecure option
guischguardian Oct 16, 2025
082e59e
doc: add changelog entry for --insecure option
guischguardian Oct 16, 2025
ceab4da
fix(client): check both insecure and allow_self_signed flags
guischguardian Oct 16, 2025
12e9d66
fix(client): correct documentation link in SSL warning message
guischguardian Oct 16, 2025
04c9810
refactor(client): use ui.display_warning instead of logger.warning
guischguardian Oct 16, 2025
19ed2a2
refactor(cli): update --insecure help text to match curl's wording
guischguardian Oct 16, 2025
510fd39
feat(cli): add deprecation warning for --allow-self-signed option
guischguardian Oct 16, 2025
c973036
feat(config): add deprecation warning for allow_self_signed config key
guischguardian Oct 16, 2025
2e88c0e
doc: update changelog to mention deprecation of allow_self_signed
guischguardian Oct 16, 2025
0855177
style: apply pre-commit formatting fixes
guischguardian Oct 16, 2025
8b5bcd3
test: add coverage for allow_self_signed config deprecation warning
guischguardian Oct 16, 2025
5f34673
refactor: implement review feedback - simplify SSL verification config
guischguardian Oct 17, 2025
9deb1cf
fix: replace all allow_self_signed usages with insecure
guischguardian Oct 17, 2025
3e667d9
fix: ensure --allow-self-signed works when placed before subcommand
guischguardian Oct 17, 2025
8983a6e
doc: update documentation link for SSL verification
guischguardian Oct 17, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<!--
A new scriv changelog fragment.

Uncomment the section that is right (remove the HTML comment wrapper).
For top level release notes, leave all the headers commented out.
-->

### Added

- 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.
- 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).

### Deprecated

- 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.

### Security

- 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.

<!--
### Changed

- A bullet item for the Changed category.

-->
<!--
### Removed

- A bullet item for the Removed category.

-->
<!--
### Deprecated

- A bullet item for the Deprecated category.

-->
<!--
### Fixed

- A bullet item for the Fixed category.

-->
9 changes: 5 additions & 4 deletions ggshield/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ def cli(
ctx: click.Context,
*,
allow_self_signed: Optional[bool],
insecure: Optional[bool],
Copy link

Choose a reason for hiding this comment

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

Bug: Insecure Parameter Mismatch

The cli function's signature now expects insecure, but the --allow-self-signed option is still active. When used, Click attempts to pass an allow_self_signed parameter, causing a TypeError since the function no longer accepts it.

Fix in Cursor Fix in Web

config_path: Optional[Path],
**kwargs: Any,
) -> None:
Expand All @@ -97,11 +98,11 @@ def cli(
elif user_config.verbose and ui.get_level() < ui.Level.VERBOSE:
ensure_level(ui.Level.VERBOSE)

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

ctx_obj.config._dotenv_vars = load_dot_env()

Expand Down
2 changes: 1 addition & 1 deletion ggshield/cmd/auth/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def token_login(config: Config, instance: Optional[str]) -> None:
client = create_client(
api_key=token,
api_url=config.api_url,
allow_self_signed=config.user_config.allow_self_signed,
allow_self_signed=config.user_config.insecure,
)
try:
response = client.get(endpoint="token")
Expand Down
2 changes: 1 addition & 1 deletion ggshield/cmd/auth/logout.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def revoke_token(config: Config, instance_url: str) -> None:
client = create_client(
token,
dashboard_to_api_url(instance_url),
allow_self_signed=config.user_config.allow_self_signed,
allow_self_signed=config.user_config.insecure,
)
try:
response = client.post(endpoint="token/revoke")
Expand Down
23 changes: 21 additions & 2 deletions ggshield/cmd/utils/common_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,30 @@ def log_file_callback(
)


def allow_self_signed_callback(
ctx: click.Context, param: click.Parameter, value: Optional[bool]
) -> Optional[bool]:
if value:
ui.display_warning(
"The --allow-self-signed option is deprecated. Use --insecure instead."
)
return create_config_callback("insecure")(ctx, param, value)


_allow_self_signed_option = click.option(
"--allow-self-signed",
is_flag=True,
default=None,
help="Ignore ssl verification.",
callback=create_config_callback("allow_self_signed"),
help="Deprecated: use --insecure.",
callback=allow_self_signed_callback,
)

_insecure_option = click.option(
"--insecure",
is_flag=True,
default=None,
help="Skip all certificate verification checks. WARNING: this option makes the transfer insecure.",
callback=create_config_callback("insecure"),
)

_check_for_updates = click.option(
Expand Down Expand Up @@ -174,6 +192,7 @@ def decorator(cmd: AnyFunction) -> AnyFunction:
_debug_option(cmd)
_log_file_option(cmd)
_allow_self_signed_option(cmd)
_insecure_option(cmd)
_check_for_updates(cmd)
return cmd

Expand Down
13 changes: 12 additions & 1 deletion ggshield/core/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from pygitguardian.models import APITokensResponse, Detail, TokenScope
from requests import Session

from . import ui
from .config import Config
from .constants import DEFAULT_INSTANCE_URL
from .errors import (
Expand Down Expand Up @@ -50,7 +51,7 @@ def create_client_from_config(config: Config) -> GGClient:
return create_client(
api_key,
api_url,
allow_self_signed=config.user_config.allow_self_signed,
allow_self_signed=config.user_config.insecure,
callbacks=callbacks,
)

Expand Down Expand Up @@ -84,6 +85,16 @@ def create_client(
def create_session(allow_self_signed: bool = False) -> Session:
session = Session()
if allow_self_signed:
ui.display_warning(
"SSL verification is disabled. Your connection to the GitGuardian API is NOT encrypted "
"and is vulnerable to man-in-the-middle attacks. Traffic, including API keys and scan results, "
"can be intercepted and modified."
)
ui.display_warning(
"To securely use self-signed certificates with Python >= 3.10, disable this option and "
"install your certificate in your system's trust store. "
"See: https://docs.gitguardian.com/ggshield-docs/configuration#allow_self_signed"
)
urllib3.disable_warnings()
session.verify = False
return session
Expand Down
13 changes: 12 additions & 1 deletion ggshield/core/config/user_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class UserConfig(FilteredConfig):
instance: Optional[str] = None
exit_zero: bool = False
verbose: bool = False
allow_self_signed: bool = False
insecure: bool = False
max_commits_for_hook: int = 50
secret: SecretConfig = field(default_factory=SecretConfig)
debug: bool = False
Expand Down Expand Up @@ -198,6 +198,7 @@ def _load_config_dict(
if dash_keys:
_warn_about_dash_keys(config_path, dash_keys)
_fix_ignore_known_secrets(dct)
_fix_allow_self_signed(dct, config_path)
elif config_version == 1:
deprecation_messages.append(
f"{config_path} uses a deprecated configuration file format."
Expand Down Expand Up @@ -226,6 +227,16 @@ def _fix_ignore_known_secrets(data: Dict[str, Any]) -> None:
secret_dct[_IGNORE_KNOWN_SECRETS_KEY] = value


def _fix_allow_self_signed(data: Dict[str, Any], config_path: Path) -> None:
"""Convert allow_self_signed to insecure and display a deprecation warning."""
if insecure := data.pop("allow_self_signed", None):
ui.display_warning(
f"{config_path}: The 'allow_self_signed' option is deprecated. "
"Use 'insecure: true' instead."
)
data["insecure"] = insecure

Copy link

Choose a reason for hiding this comment

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

Bug: Self-Signed Override Ignores User Config

The _fix_allow_self_signed function unconditionally overwrites the insecure configuration setting when allow_self_signed is present. This allows the deprecated allow_self_signed value to override an explicitly set insecure value, ignoring the user's intended configuration.

Fix in Cursor Fix in Web


def _warn_about_dash_keys(config_path: Path, dash_keys: Set[str]) -> None:
for old_key in sorted(dash_keys):
new_key = old_key.replace("-", "_")
Expand Down
2 changes: 1 addition & 1 deletion ggshield/core/config/v1_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def copy_if_set(dct: Dict[str, Any], dst_key: str, src_key: Optional[str] = None
copy_if_set(dct, "instance")
copy_if_set(dct, "exit_zero")
copy_if_set(dct, "verbose")
copy_if_set(dct, "allow_self_signed")
copy_if_set(dct, "insecure", "allow_self_signed")
copy_if_set(dct, "max_commits_for_hook")

return dct
Expand Down
4 changes: 2 additions & 2 deletions ggshield/verticals/auth/oauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def _claim_token(self, authorization_code: str) -> None:
body=urlparse.urlencode(request_params),
)

session = create_session(self.config.user_config.allow_self_signed)
session = create_session(self.config.user_config.insecure)
response = session.post(
urljoin(self.api_url, "/v1/oauth/token"),
request_body,
Expand Down Expand Up @@ -265,7 +265,7 @@ def _validate_access_token(self) -> Dict[str, Any]:
response = create_client(
self._access_token,
self.api_url,
allow_self_signed=self.config.user_config.allow_self_signed,
allow_self_signed=self.config.user_config.insecure,
).get(endpoint="token")
if not response.ok:
raise OAuthError("The created token is invalid.")
Expand Down
2 changes: 1 addition & 1 deletion ggshield/verticals/hmsl/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def get_token(config: Config) -> Optional[str]:
client = create_client(
api_url=config.saas_api_url,
api_key=config.saas_api_key,
allow_self_signed=config.user_config.allow_self_signed,
allow_self_signed=config.user_config.insecure,
)
audience = config.hmsl_audience
except (MissingTokenError, AuthExpiredError):
Expand Down
7 changes: 6 additions & 1 deletion tests/unit/cmd/scan/test_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,15 @@ def test_instance_option(self, cli_fs_runner):

@pytest.mark.parametrize("position", [0, 1, 2, 3, 4])
def test_ssl_verify(self, cli_fs_runner, position):
"""
GIVEN the --insecure flag
WHEN running the path scan command
THEN SSL verification is disabled
"""
self.create_files()

cmd = ["secret", "scan", "path", "file1"]
cmd.insert(position, "--allow-self-signed")
cmd.insert(position, "--insecure")

with patch("ggshield.core.client.GGClient") as client_mock:
cli_fs_runner.invoke(cli, cmd)
Expand Down
26 changes: 25 additions & 1 deletion tests/unit/cmd/test_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,38 @@ def get_api_status(env, instance=None):

@pytest.mark.parametrize("verify", [True, False])
def test_ssl_verify(cli_fs_runner, verify):
cmd = ["api-status"] if verify else ["--allow-self-signed", "api-status"]
"""
GIVEN the --insecure flag
WHEN running the api-status command
THEN SSL verification is disabled
"""
cmd = ["api-status"] if verify else ["--insecure", "api-status"]

with mock.patch("ggshield.core.client.GGClient") as client_mock:
cli_fs_runner.invoke(cli, cmd)
_, kwargs = client_mock.call_args
assert kwargs["session"].verify == verify


@pytest.mark.parametrize(
"cmd",
[
["api-status", "--allow-self-signed"],
["--allow-self-signed", "api-status"],
],
)
def test_allow_self_signed_backward_compatibility(cli_fs_runner, cmd):
"""
GIVEN the deprecated --allow-self-signed flag
WHEN it's placed before or after the subcommand
THEN SSL verification is disabled in both cases (backward compatibility)
"""
with mock.patch("ggshield.core.client.GGClient") as client_mock:
cli_fs_runner.invoke(cli, cmd)
_, kwargs = client_mock.call_args
assert kwargs["session"].verify is False


@pytest.mark.parametrize("command", ["api-status", "quota"])
def test_instance_option(cli_fs_runner, command):
"""
Expand Down
20 changes: 20 additions & 0 deletions tests/unit/core/config/test_user_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,26 @@ def test_can_load_ignored_known_secrets_from_secret(self, local_config_path):
config, _ = UserConfig.load(local_config_path)
assert config.secret.ignore_known_secrets

def test_allow_self_signed_deprecation_warning(self, local_config_path, capsys):
"""
GIVEN a config file containing allow_self_signed: true
WHEN loading the config
THEN it is converted to insecure and a deprecation warning is displayed
"""
write_yaml(
local_config_path,
{
"version": 2,
"allow_self_signed": True,
},
)
config, _ = UserConfig.load(local_config_path)
assert config.insecure is True
captured = capsys.readouterr()
assert "allow_self_signed" in captured.err
assert "deprecated" in captured.err
assert "insecure" in captured.err

def test_bad_local_config(self, local_config_path, global_config_path):
"""
GIVEN a malformed .gitguardian.yaml, with a list of instance
Expand Down