Skip to content

feat(proxy): auto trust if possible#431

Merged
jdx merged 1 commit into
jdx:mainfrom
gaojunran:feat-proxy-trust
May 11, 2026
Merged

feat(proxy): auto trust if possible#431
jdx merged 1 commit into
jdx:mainfrom
gaojunran:feat-proxy-trust

Conversation

@gaojunran

Copy link
Copy Markdown
Contributor

No description provided.

@greptile-apps

greptile-apps Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR centralizes all CA certificate trust management into src/proxy/trust.rs and adds two features: a proxy untrust subcommand to remove the CA from the system trust store, and an auto_trust mechanism that installs the CA automatically when the supervisor starts (enabled by default via proxy.auto_trust = true).

  • The new trust.rs module fixes several issues from earlier implementations: the macOS verify-cert check now omits the -L -p ssl policy flags (which caused false negatives for CA certs), stdout/stderr from security verify-cert is suppressed, and the Arch Linux update command is correctly split into ["trust", "extract-compat"].
  • install_cert_linux now rolls back the copied cert file if the update command fails, preventing a stuck is_ca_trusted_linux state.
  • The uninstall_cert_macos path dynamically extracts the CN from the cert using openssl instead of hardcoding "Pitchfork Local CA", and skips keychain deletion if extraction fails rather than deleting the wrong entry.

Confidence Score: 5/5

The core trust install/uninstall paths are structurally sound with good rollback handling; the two findings are minor edge cases in error paths that do not affect the happy path.

The auto-trust and untrust logic is well-structured with proper rollback on Linux install failure and correct macOS CN extraction. The two issues found are both in error-handling or diagnostic-output paths: one leaks security error text to the terminal during untrust (cosmetic), and one produces a false-positive trust result only if the cert file exists on disk but cannot be read (a very unusual failure mode). Neither affects the primary install/uninstall flow.

src/proxy/trust.rs — the delete-certificate loop stdout/stderr and the unwrap_or_default() content-comparison in is_ca_trusted_linux

Important Files Changed

Filename Overview
src/proxy/trust.rs New trust management module consolidating is_ca_trusted, install_cert, uninstall_cert, and auto_trust; macOS verify-cert and Arch Linux command fixes are applied; minor: delete-certificate stdout/stderr not suppressed; is_ca_trusted_linux false-positive possible if both file reads return empty bytes
src/cli/proxy.rs Adds Untrust subcommand (delegates to trust::uninstall_cert), moves install logic to trust.rs, adds trust status display to proxy status; logic is straightforward delegation
src/supervisor/mod.rs Adds auto_trust call during supervisor startup when proxy.auto_trust=true; result is non-fatal, errors logged as warnings
src/cli/supervisor/start.rs Adjusts trust warning to only fire when cert exists but is not trusted (previously warned when cert didn't exist)
src/proxy/mod.rs Exports the new trust module
settings.toml Adds proxy.auto_trust Bool setting (default true) with PITCHFORK_PROXY_AUTO_TRUST env var
docs/cli/proxy/untrust.md New doc page for proxy untrust command, auto-generated from usage spec
docs/guides/port-management.md Updated docs to reflect auto-trust behavior and new proxy untrust command

Reviews (8): Last reviewed commit: "feat(proxy): try auto trust" | Re-trigger Greptile

Comment thread src/cli/proxy.rs
Comment thread src/proxy/trust.rs
Comment thread src/proxy/trust.rs Outdated
Comment thread src/proxy/trust.rs Outdated
@gaojunran gaojunran force-pushed the feat-proxy-trust branch from 360f4d2 to a85fcc1 Compare May 8, 2026 12:10
@gaojunran gaojunran marked this pull request as ready for review May 8, 2026 12:20
@gaojunran gaojunran changed the title feat(proxy): try auto trust feat(proxy): auto trust when possible May 8, 2026
@gaojunran gaojunran changed the title feat(proxy): auto trust when possible feat(proxy): auto trust if possible May 8, 2026
Comment thread src/proxy/trust.rs Outdated
Comment thread src/proxy/trust.rs Outdated
@gaojunran gaojunran marked this pull request as draft May 10, 2026 14:40
@gaojunran

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread src/proxy/trust.rs
Comment thread src/proxy/trust.rs Outdated
@gaojunran

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread src/proxy/trust.rs
@gaojunran

Copy link
Copy Markdown
Contributor Author

@greptileai

@gaojunran

Copy link
Copy Markdown
Contributor Author

@jdx ready for review!

@gaojunran gaojunran marked this pull request as ready for review May 11, 2026 06:36
@jdx jdx merged commit 12e758b into jdx:main May 11, 2026
4 checks passed
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.

2 participants