Skip to content

Commit e30f8ae

Browse files
committed
fix(cli): accept --auth=false / --auth false (and same for every bool flag)
Operator feedback: someone tried `--auth=false` and `--auth false` to explicitly disable auth, both errored with "unexpected value 'false' for '--auth' found". They then read the policy error message — which described state as "auth=false on a non-loopback bind ..." — and reasonably mistook that for a CLI hint. Two changes to make the surface match the expectation. 1. Every `bool` flag in `Config` now accepts: - `--flag` (bare, resolves to true) - `--flag=true` - `--flag=false` - `--flag true` - `--flag false` Absence still falls through to `default_value_t = false`. Implementation: clap's default `bool` action is `SetTrue`, which is the bare-`--flag`-only form. Switching to `ArgAction::Set` with `num_args = 0..=1` + `default_missing_value = "true"` keeps the bare form working and adds the explicit-value forms most operators expect from kubectl/helm. Applied uniformly to: `trino_ssl`, `trino_tls_no_verify`, `trino_allow_plaintext_auth`, `auth`, `allow_insecure_listener`. 2. The policy error for "auth disabled on non-loopback bind" used to open with the phrase `--auth=false on a non-loopback bind ...`, which reads as if `--auth=false` is a valid CLI flag (the source of the original confusion). Reworded to describe the state in plain English and lay out the three opt-in choices (loopback, `--auth` + TLS, `--allow-insecure-listener`) as a numbered list so the user can pick directly. No information lost; clarity added. Verified manually with the gateway binary: all five `--auth` forms parse, both with and without the surrounding policy gates firing. Existing policy unit tests (191 passing) unchanged.
1 parent 5b544f3 commit e30f8ae

2 files changed

Lines changed: 26 additions & 10 deletions

File tree

src/config.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,19 @@
22
// SPDX-License-Identifier: OSL-3.0
33
use std::path::PathBuf;
44

5-
use clap::Parser;
5+
use clap::{ArgAction, Parser};
6+
7+
// clap's default action for `bool` is `SetTrue`: only the bare `--flag`
8+
// form is accepted, and `--flag=false` errors out. That's surprising for
9+
// operators used to `kubectl`/`helm`-style explicit-value flags. Every
10+
// boolean field below uses `num_args=0..=1` + `default_missing_value="true"`
11+
// + `ArgAction::Set` so all of these work:
12+
// --flag (bare; resolves to true via default_missing_value)
13+
// --flag=true
14+
// --flag=false
15+
// --flag true (space-separated)
16+
// --flag false
17+
// Absence falls through to `default_value_t = false`.
618

719
/// PostgreSQL-to-Trino gateway configuration.
820
// WARNING: Debug is derived for clap compatibility. If credential fields
@@ -47,27 +59,27 @@ pub struct Config {
4759
pub trino_user: String,
4860

4961
/// Use HTTPS to connect to Trino.
50-
#[arg(long, default_value_t = false)]
62+
#[arg(long, default_value_t = false, num_args = 0..=1, default_missing_value = "true", action = ArgAction::Set)]
5163
pub trino_ssl: bool,
5264

5365
/// Skip TLS certificate-chain and hostname verification on the Trino
5466
/// connection. Useful with self-signed certs in a trusted network.
5567
/// Only meaningful with `--trino-ssl`.
56-
#[arg(long, default_value_t = false)]
68+
#[arg(long, default_value_t = false, num_args = 0..=1, default_missing_value = "true", action = ArgAction::Set)]
5769
pub trino_tls_no_verify: bool,
5870

5971
/// Allow forwarding the PG client's password to Trino over plain
6072
/// HTTP. Required when `--auth` is on and Trino is reached over HTTP
6173
/// (`--trino-ssl=false`); without it the Trino client refuses to send
6274
/// credentials. The password crosses the network in cleartext, so use
6375
/// only with a loopback or otherwise-trusted Trino endpoint.
64-
#[arg(long, default_value_t = false)]
76+
#[arg(long, default_value_t = false, num_args = 0..=1, default_missing_value = "true", action = ArgAction::Set)]
6577
pub trino_allow_plaintext_auth: bool,
6678

6779
/// Require password authentication from PG clients.
6880
/// Credentials are forwarded to Trino as HTTP Basic auth.
6981
/// When disabled, connects to Trino with the --trino-user and no password.
70-
#[arg(long, default_value_t = false)]
82+
#[arg(long, default_value_t = false, num_args = 0..=1, default_missing_value = "true", action = ArgAction::Set)]
7183
pub auth: bool,
7284

7385
/// Acknowledge that the gateway should accept unauthenticated PG
@@ -76,7 +88,7 @@ pub struct Config {
7688
/// every network-reachable client gets unauthenticated access to
7789
/// Trino as `--trino-user`. Use only when Trino itself enforces
7890
/// authentication or the network is otherwise trusted.
79-
#[arg(long, default_value_t = false)]
91+
#[arg(long, default_value_t = false, num_args = 0..=1, default_missing_value = "true", action = ArgAction::Set)]
8092
pub allow_insecure_listener: bool,
8193

8294
/// Maximum number of concurrent PG client connections. Excess

src/policy.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,14 @@ pub fn classify(config: &Config) -> Result<AuthPosture> {
9595
return Ok(AuthPosture::DisabledOpenBind);
9696
}
9797
bail!(
98-
"--auth=false on a non-loopback bind ({}) is not allowed by default. \
99-
Either pass --auth (and --tls-cert/--tls-key for password protection) \
100-
to require authentication, or pass --allow-insecure-listener to \
101-
explicitly allow unauthenticated network access.",
98+
"auth is disabled and the listen address ({}) is not loopback. \
99+
That combination would expose Trino to every network-reachable client \
100+
without authentication. Pick one of the following: \
101+
(a) bind to 127.0.0.1 / [::1] / localhost for local development; \
102+
(b) enable auth with `--auth` plus TLS via `--tls-cert` and `--tls-key`; \
103+
(c) explicitly opt in to unauthenticated network access with \
104+
`--allow-insecure-listener` (only when Trino itself authenticates \
105+
or the network is otherwise trusted).",
102106
config.listen_addr
103107
)
104108
}

0 commit comments

Comments
 (0)