feat: add --insecure option and deprecate --allow-self-signed #1132

guischguardian · 2025-10-16T10:17:52Z

Context

We received a report about ggshield's --allow-self-signed option. The option name is misleading: it suggests that self-signed certificates are validated, when in reality it completely disables SSL verification, making connections vulnerable to man-in-the-middle attacks.

This PR addresses the issue by:

Adding a clearer --insecure option (matching curl's convention)
Deprecating --allow-self-signed with prominent warnings
Adding security warnings explaining the risks and secure alternatives

See SCRT-5952.

What has been done

New Features

Added --insecure CLI option and insecure configuration setting as clearer alternatives to --allow-self-signed
Added prominent warning messages when SSL verification is disabled (via either option), explaining:
- The security risks (MITM attacks, traffic interception)
- The recommended secure alternative (system certificate trust store with Python >= 3.10)
- Link to documentation for more information
Help text for --insecure matches curl's standard wording for consistency

Deprecation

The --allow-self-signed CLI option is now deprecated with a warning displayed on every use
The allow_self_signed config setting is now deprecated with a warning displayed when loading config files
Both options remain fully functional for backward compatibility
Deprecation warnings guide users to use --insecure instead

Implementation Details

Both --insecure and --allow-self-signed set both internal flags for consistency
Warnings use ui.display_warning() for proper user-facing output (not logger.warning())
Comprehensive test coverage for both old and new options

Validation

To validate the new --insecure option:

# Test the new --insecure flag works
ggshield --insecure secret scan path .

# Verify deprecation warning is shown for --allow-self-signed
ggshield --allow-self-signed secret scan path .

Expected behavior:

Both commands disable SSL verification
--allow-self-signed displays a deprecation warning guiding to --insecure
Both display security warnings about MITM risks and secure alternatives

To validate config file deprecation:

# Create a config with allow_self_signed
echo "version: 2" > .gitguardian.yaml
echo "allow_self_signed: true" >> .gitguardian.yaml

# Run any ggshield command
ggshield secret scan path .

Expected: Deprecation warning about allow_self_signed config key

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.

Add the 'insecure' configuration field alongside 'allow_self_signed' to prepare for introducing the --insecure flag with clearer semantics. Issue: SCRT-5952

Add the --insecure flag to clearly communicate that SSL verification is being disabled, making the connection vulnerable to MITM attacks. This option is clearer than --allow-self-signed about what it does. Issue: SCRT-5952

Display clear warnings when SSL verification is disabled, explaining: - The security risks (MITM attacks, interception) - The recommended secure alternative (system trust store with Python >= 3.10) - A link to documentation for more information This warning appears every time a session is created with SSL verification disabled, whether through --allow-self-signed or --insecure. Issue: SCRT-5952

Update the main CLI handler to accept both --insecure and --allow-self-signed options. When either is set, both flags are enabled in the config to maintain consistency and ensure they work as aliases. This ensures backward compatibility while providing a clearer option name. Issue: SCRT-5952

Add test coverage for the new --insecure flag to ensure it works correctly and disables SSL verification as expected, matching the behavior of --allow-self-signed. Issue: SCRT-5952

Document the addition of the --insecure option and improved security warnings when SSL verification is disabled. Issue: SCRT-5952

Update create_client_from_config to check both the insecure and allow_self_signed flags when creating the API client. This ensures that either flag will correctly disable SSL verification. Issue: SCRT-5952

Fix the URL in the warning message to point to the correct documentation page (remove 'reference/' from the path). Issue: SCRT-5952

Switch from logger.warning to ui.display_warning for SSL verification warnings to properly display messages to users through ggshield's UI system rather than the logging system. This ensures warnings are properly formatted and visible to end users regardless of their logging configuration. Issue: SCRT-5952

Use curl's standard wording for the --insecure option to maintain consistency with widely-used tools and make the security implications immediately clear to users familiar with curl. Issue: SCRT-5952

Display a deprecation warning when --allow-self-signed is used, guiding users to use --insecure instead, which more accurately describes the behavior (disabling all certificate validation). The option remains functional for backward compatibility but now clearly indicates it will be removed in a future version. Issue: SCRT-5952

Display a deprecation warning when 'allow_self_signed' is used in configuration files, guiding users to use 'insecure' instead. The option remains functional for backward compatibility but now clearly indicates it will be removed in a future version. Issue: SCRT-5952

Add a Deprecated section to the changelog documenting that allow_self_signed is now deprecated in favor of the insecure option. Issue: SCRT-5952

codecov · 2025-10-16T10:20:00Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.98%. Comparing base (b91c894) to head (8983a6e).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1132      +/-   ##
==========================================
+ Coverage   91.95%   91.98%   +0.02%     
==========================================
  Files         144      144              
  Lines        6166     6188      +22     
==========================================
+ Hits         5670     5692      +22     
  Misses        496      496

Flag	Coverage Δ
unittests	`91.98% <100.00%> (+0.02%)`	⬆️

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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Apply black formatting and fix end-of-file for changelog. Issue: SCRT-5952

Add test to verify that the deprecation warning is properly added to deprecation_messages when allow_self_signed is used in config files. This brings the patch coverage to 100%. Issue: SCRT-5952

ggshield/cmd/utils/common_options.py

ggshield/core/config/user_config.py

@agateau-gg

Implement all review feedback from @agateau-gg: - Remove allow_self_signed field from UserConfig, keep only insecure - Convert allow_self_signed to insecure during config loading - Use ui.display_warning() directly instead of deprecation_messages - Shorten all deprecation warning messages - Reorder --insecure help text (functionality before warning) - Update v1 config converter to map allow_self_signed to insecure - Simplify code by eliminating dual field state This makes the code cleaner and prevents inconsistent state where allow_self_signed and insecure could have different values. Issue: SCRT-5952

Update remaining references to allow_self_signed attribute in: - ggshield/verticals/hmsl/utils.py - ggshield/verticals/auth/oauth.py - ggshield/cmd/auth/logout.py - ggshield/cmd/auth/login.py Issue: SCRT-5952

agateau-gg

One last change and this can go in.

ggshield/__main__.py

The cli() function needs to accept the allow_self_signed parameter to handle the case where --allow-self-signed is placed before the subcommand (e.g., 'ggshield --allow-self-signed api-status'). Without this, the callback can't update the config at that stage because ctx.obj is None, breaking backward compatibility. Added test to verify both placements work: - ggshield api-status --allow-self-signed ✓ - ggshield --allow-self-signed api-status ✓ Issue: SCRT-5952

Updated the documentation link in the SSL verification warning message to point to the correct anchor in the public documentation. The link now points to the 'support-for-self-signed-certificates' section instead of 'allow_self_signed'. Issue: SCRT-5952

guischguardian added 13 commits October 16, 2025 10:07

feat(config): add insecure field to UserConfig

2bbfef3

Add the 'insecure' configuration field alongside 'allow_self_signed' to prepare for introducing the --insecure flag with clearer semantics. Issue: SCRT-5952

feat(cli): add --insecure option as an explicit alternative

a54458f

Add the --insecure flag to clearly communicate that SSL verification is being disabled, making the connection vulnerable to MITM attacks. This option is clearer than --allow-self-signed about what it does. Issue: SCRT-5952

test: add tests for --insecure option

f3faf06

Add test coverage for the new --insecure flag to ensure it works correctly and disables SSL verification as expected, matching the behavior of --allow-self-signed. Issue: SCRT-5952

doc: add changelog entry for --insecure option

082e59e

Document the addition of the --insecure option and improved security warnings when SSL verification is disabled. Issue: SCRT-5952

fix(client): check both insecure and allow_self_signed flags

ceab4da

Update create_client_from_config to check both the insecure and allow_self_signed flags when creating the API client. This ensures that either flag will correctly disable SSL verification. Issue: SCRT-5952

fix(client): correct documentation link in SSL warning message

12e9d66

Fix the URL in the warning message to point to the correct documentation page (remove 'reference/' from the path). Issue: SCRT-5952

refactor(cli): update --insecure help text to match curl's wording

19ed2a2

Use curl's standard wording for the --insecure option to maintain consistency with widely-used tools and make the security implications immediately clear to users familiar with curl. Issue: SCRT-5952

doc: update changelog to mention deprecation of allow_self_signed

2e88c0e

Add a Deprecated section to the changelog documenting that allow_self_signed is now deprecated in favor of the insecure option. Issue: SCRT-5952

guischguardian assigned agateau-gg Oct 16, 2025

guischguardian added 2 commits October 16, 2025 14:08

style: apply pre-commit formatting fixes

0855177

Apply black formatting and fix end-of-file for changelog. Issue: SCRT-5952

test: add coverage for allow_self_signed config deprecation warning

8b5bcd3

Add test to verify that the deprecation warning is properly added to deprecation_messages when allow_self_signed is used in config files. This brings the patch coverage to 100%. Issue: SCRT-5952

guischguardian marked this pull request as ready for review October 16, 2025 12:20

guischguardian requested a review from a team as a code owner October 16, 2025 12:20

agateau-gg requested changes Oct 17, 2025

View reviewed changes

guischguardian added 2 commits October 17, 2025 14:44

fix: replace all allow_self_signed usages with insecure

9deb1cf

Update remaining references to allow_self_signed attribute in: - ggshield/verticals/hmsl/utils.py - ggshield/verticals/auth/oauth.py - ggshield/cmd/auth/logout.py - ggshield/cmd/auth/login.py Issue: SCRT-5952

This comment was marked as outdated.

Sign in to view

agateau-gg requested changes Oct 17, 2025

View reviewed changes

ggshield/__main__.py Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

agateau-gg approved these changes Oct 21, 2025

View reviewed changes

agateau-gg merged commit e5aa4a9 into main Oct 21, 2025
31 checks passed

agateau-gg deleted the gcasier/SCRT-5952/add_insecure_option branch October 21, 2025 15:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add --insecure option and deprecate --allow-self-signed #1132

feat: add --insecure option and deprecate --allow-self-signed #1132

Uh oh!

guischguardian commented Oct 16, 2025

Uh oh!

codecov bot commented Oct 16, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

agateau-gg left a comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat: add --insecure option and deprecate --allow-self-signed #1132

feat: add --insecure option and deprecate --allow-self-signed #1132

Uh oh!

Conversation

guischguardian commented Oct 16, 2025

Context

What has been done

New Features

Deprecation

Implementation Details

Validation

PR check list

Uh oh!

codecov bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

agateau-gg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Oct 16, 2025 •

edited

Loading