diff --git a/cmd/cli/cli.go b/cmd/cli/cli.go index c497678..98dc763 100644 --- a/cmd/cli/cli.go +++ b/cmd/cli/cli.go @@ -385,7 +385,7 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { // would silently re-enable interception from config. if updateConfigInterceptMode(&cfg, interceptMode) { updated = true - mainLog.Load().Info().Msgf("writing intercept_mode = %q to config", cfg.Service.InterceptMode) + mainLog.Load().Info().Msgf("writing intercept_mode = %q to config (requested %q)", cfg.Service.InterceptMode, interceptMode) } if updated { diff --git a/cmd/cli/commands.go b/cmd/cli/commands.go index b58ae78..6ebf5c7 100644 --- a/cmd/cli/commands.go +++ b/cmd/cli/commands.go @@ -275,7 +275,7 @@ func initRunCmd() *cobra.Command { _ = runCmd.Flags().MarkHidden("iface") runCmd.Flags().StringVarP(&cdUpstreamProto, "proto", "", ctrld.ResolverTypeDOH, `Control D upstream type, either "doh" or "doh3"`) runCmd.Flags().BoolVarP(&rfc1918, "rfc1918", "", false, "Listen on RFC1918 addresses when 127.0.0.1 is the only listener") - runCmd.Flags().StringVarP(&interceptMode, "intercept-mode", "", "", "OS-level DNS interception mode: 'dns' (with VPN split routing) or 'hard' (all DNS through ctrld, no VPN split routing)") + runCmd.Flags().StringVarP(&interceptMode, "intercept-mode", "", "", "OS-level DNS interception mode: 'off' (disable interception and clear a persisted intercept_mode), 'dns' (with VPN split routing), or 'hard' (all DNS through ctrld, no VPN split routing)") runCmd.FParseErrWhitelist = cobra.FParseErrWhitelist{UnknownFlags: true} rootCmd.AddCommand(runCmd) @@ -394,8 +394,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c svcExists := serviceConfigFileExists() mainLog.Load().Debug().Msgf("intercept upgrade check: args=%v interceptOnly=%v svcConfigExists=%v interceptMode=%q", osArgsEarly, interceptOnly, svcExists, interceptMode) if interceptOnly && svcExists { - // Replace any existing split or --intercept-mode= form. Keep an - // explicit "off" argument so it overrides a previously persisted config + // An explicit "off" argument must override a previously persisted config // value while the service clears that value on startup. if err := removeServiceFlag("--intercept-mode"); err != nil { mainLog.Load().Fatal().Err(err).Msg("failed to remove existing intercept mode from service arguments") @@ -778,7 +777,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c startCmd.Flags().BoolVarP(&startOnly, "start_only", "", false, "Do not install new service") _ = startCmd.Flags().MarkHidden("start_only") startCmd.Flags().BoolVarP(&rfc1918, "rfc1918", "", false, "Listen on RFC1918 addresses when 127.0.0.1 is the only listener") - startCmd.Flags().StringVarP(&interceptMode, "intercept-mode", "", "", "OS-level DNS interception mode: 'dns' (with VPN split routing) or 'hard' (all DNS through ctrld, no VPN split routing)") + startCmd.Flags().StringVarP(&interceptMode, "intercept-mode", "", "", "OS-level DNS interception mode: 'off' (disable interception and clear a persisted intercept_mode), 'dns' (with VPN split routing), or 'hard' (all DNS through ctrld, no VPN split routing)") routerCmd := &cobra.Command{ Use: "setup", diff --git a/cmd/cli/intercept_mode_config_test.go b/cmd/cli/intercept_mode_config_test.go index a3167eb..9d0fe4d 100644 --- a/cmd/cli/intercept_mode_config_test.go +++ b/cmd/cli/intercept_mode_config_test.go @@ -36,3 +36,30 @@ func TestUpdateConfigInterceptMode(t *testing.T) { }) } } + +func TestConfiguredInterceptMode(t *testing.T) { + oldInterceptMode := interceptMode + t.Cleanup(func() { interceptMode = oldInterceptMode }) + + p := &prog{cfg: &ctrld.Config{}} + p.cfg.Service.InterceptMode = "dns" + + tests := []struct { + name string + flag string + want string + }{ + {name: "empty flag falls back to config", flag: "", want: "dns"}, + {name: "explicit off is final", flag: "off", want: "off"}, + {name: "explicit hard wins over config", flag: "hard", want: "hard"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + interceptMode = tc.flag + if got := p.configuredInterceptMode(); got != tc.want { + t.Fatalf("configuredInterceptMode() = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/cmd/cli/main.go b/cmd/cli/main.go index 762777d..7e6dd8d 100644 --- a/cmd/cli/main.go +++ b/cmd/cli/main.go @@ -42,7 +42,7 @@ var ( cleanup bool startOnly bool rfc1918 bool - interceptMode string // "", "dns", or "hard" — set via --intercept-mode flag or config + interceptMode string // "", "off", "dns", or "hard" — set via --intercept-mode flag or config dnsIntercept bool // derived: interceptMode == "dns" || interceptMode == "hard" hardIntercept bool // derived: interceptMode == "hard" diff --git a/cmd/cli/prog.go b/cmd/cli/prog.go index a51417c..02a9405 100644 --- a/cmd/cli/prog.go +++ b/cmd/cli/prog.go @@ -1091,11 +1091,11 @@ func (p *prog) setDNS() { } // configuredInterceptMode resolves the service's effective intercept mode without -// mutating package state. Platform startup preflights use the same precedence as -// setDNS so they do not make adapter-DNS decisions from a different mode value. +// mutating package state. An explicit flag value, including "off", takes priority +// over the persisted config value. func (p *prog) configuredInterceptMode() string { im := interceptMode - if im == "" || im == "off" { + if im == "" { im = p.cfg.Service.InterceptMode } return im diff --git a/cmd/cli/service.go b/cmd/cli/service.go index 0cad94f..0925b41 100644 --- a/cmd/cli/service.go +++ b/cmd/cli/service.go @@ -162,10 +162,8 @@ func (s *systemd) Start() error { // This is necessary for running self-upgrade flow. func ensureSystemdKillMode(r io.Reader) (opts []*unit.UnitOption, change bool) { opts, err := unit.DeserializeOptions(r) - // staticcheck sees only the explicit non-nil sends on the lexer's error - // channel, so it reports this comparison as always true. On success the - // lexer sends nothing and closes the channel, so the receive yields a nil - // error and this branch is not taken. + // On success the lexer sends nothing and closes the channel, so the receive + // yields a nil error and this branch is not taken. if err != nil { mainLog.Load().Error().Err(err).Msg("failed to deserialize options") return diff --git a/cmd/cli/service_args_darwin_test.go b/cmd/cli/service_args_darwin_test.go index dc23b3b..1e535f0 100644 --- a/cmd/cli/service_args_darwin_test.go +++ b/cmd/cli/service_args_darwin_test.go @@ -15,6 +15,11 @@ func TestServiceArgumentPresent(t *testing.T) { if serviceArgumentPresent(out, "off") { t.Fatal("substring in an unrelated path was mistaken for the off argument") } + + splitOut := []byte("Array {\n /usr/local/bin/ctrld\n run\n --intercept-mode\n dns\n}\n") + if !serviceArgumentPresent(splitOut, "--intercept-mode") { + t.Fatal("standalone flag argument was not found") + } } func TestServiceFlagPosition(t *testing.T) { diff --git a/cmd/cli/service_args_windows.go b/cmd/cli/service_args_windows.go index 1eed7da..625da28 100644 --- a/cmd/cli/service_args_windows.go +++ b/cmd/cli/service_args_windows.go @@ -89,7 +89,7 @@ func verifyServiceRegistration() error { mainLog.Load().Debug().Msgf("Service registry: BinaryPathName = %q", config.BinaryPathName) // If intercept mode is set, verify the flag is present in BinPath. - if interceptMode == "dns" || interceptMode == "hard" { + if interceptMode == "off" || interceptMode == "dns" || interceptMode == "hard" { if !strings.Contains(config.BinaryPathName, "--intercept-mode") { return fmt.Errorf("service registry: --intercept-mode flag missing from BinaryPathName (expected mode %q)", interceptMode) } diff --git a/test-scripts/darwin/test-pkg-intercept-mode.sh b/test-scripts/darwin/test-pkg-intercept-mode.sh index eb06e45..d1029cd 100755 --- a/test-scripts/darwin/test-pkg-intercept-mode.sh +++ b/test-scripts/darwin/test-pkg-intercept-mode.sh @@ -2,7 +2,7 @@ set -eu repo_root=$(CDPATH= cd -- "$(dirname -- "$0")/../.." && pwd) -postinstall="$repo_root/scripts/pkg/postinstall" +postinstall_source="$repo_root/scripts/pkg/postinstall" fixture=$(mktemp -d "${TMPDIR:-/tmp}/ctrld-pkg-intercept.XXXXXX") trap 'rm -rf "$fixture"' EXIT HUP INT TERM @@ -36,16 +36,26 @@ printf 'launchctl %s\n' "$*" >>"$CALLS" exit 0 EOF +cat >"$bin/mktemp" <<'EOF' +#!/bin/sh +[ "${1:-}" = "-t" ] && [ -n "${2:-}" ] || exit 1 +path="$FAKE_TMPDIR/$2.capture" +: >"$path" +printf '%s\n' "$path" +EOF + cat >"$bin/ctrld" <<'EOF' #!/bin/sh printf 'ctrld %s\n' "$*" >>"$CALLS" -case " $* " in - *" --cd-org="*) : >"$CTRLD_POSTINSTALL_PLIST" ;; -esac -exit 0 +if [ "${FAKE_CTRLD_EXIT:-0}" = "0" ]; then + case " $* " in + *" --cd-org="*) : >"$FAKE_PLIST" ;; + esac +fi +exit "${FAKE_CTRLD_EXIT:-0}" EOF -chmod +x "$bin/defaults" "$bin/launchctl" "$bin/ctrld" +chmod +x "$bin/defaults" "$bin/launchctl" "$bin/mktemp" "$bin/ctrld" assert_contains() { expected=$1 @@ -72,31 +82,50 @@ run_case() { existing=$2 mode_present=$3 mode=$4 + ctrld_exit=${5:-0} + expected_status=${6:-0} case_dir="$fixture/$name" mkdir -p "$case_dir" plist="$case_dir/ctrld.plist" prefs="$case_dir/preferences" calls="$case_dir/calls" output="$case_dir/output" + postinstall="$case_dir/postinstall" : >"$calls" if [ "$existing" = "1" ]; then : >"$plist" fi + sed \ + -e "s|^PLIST=\"/Library/LaunchDaemons/ctrld.plist\"$|PLIST=\"$plist\"|" \ + -e "s|^CTRLD=\"/usr/local/bin/ctrld\"$|CTRLD=\"$bin/ctrld\"|" \ + -e "s|^PREFS=\"/Library/Managed Preferences/com.controld.ctrld\"$|PREFS=\"$prefs\"|" \ + "$postinstall_source" >"$postinstall" + chmod +x "$postinstall" + + status=0 PATH="$bin:$PATH" \ CALLS="$calls" \ FAKE_TOKEN_PRESENT=1 \ FAKE_TOKEN=test-token \ FAKE_MODE_PRESENT="$mode_present" \ FAKE_MODE="$mode" \ - CTRLD_POSTINSTALL_PLIST="$plist" \ - CTRLD_POSTINSTALL_CTRLD="$bin/ctrld" \ - CTRLD_POSTINSTALL_PREFS="$prefs" \ - "$postinstall" >"$output" 2>&1 + FAKE_CTRLD_EXIT="$ctrld_exit" \ + FAKE_PLIST="$plist" \ + FAKE_TMPDIR="$case_dir" \ + "$postinstall" >"$output" 2>&1 || status=$? + + if [ "$status" -ne "$expected_status" ]; then + printf 'FAIL: %s exited %s, want %s\n' "$name" "$status" "$expected_status" >&2 + sed -n '1,120p' "$output" >&2 + exit 1 + fi printf '%s\n' "$case_dir" } +assert_not_contains 'CTRLD_POSTINSTALL_' "$postinstall_source" + case_dir=$(run_case fresh-legacy 0 0 '') assert_contains 'ctrld start --cd-org=test-token' "$case_dir/calls" assert_not_contains '--intercept-mode' "$case_dir/calls" @@ -120,4 +149,18 @@ case_dir=$(run_case upgrade-intercept 1 1 intercept-dns) assert_contains 'ctrld start --intercept-mode dns' "$case_dir/calls" assert_not_contains 'launchctl load' "$case_dir/calls" +case_dir=$(run_case fresh-invalid 0 1 invalid) +assert_contains 'WARNING: unsupported InterceptMode in managed preferences; using standard mode' "$case_dir/output" +assert_not_contains '--intercept-mode' "$case_dir/calls" + +case_dir=$(run_case upgrade-invalid 1 1 invalid) +assert_contains 'WARNING: unsupported InterceptMode in managed preferences; preserving existing service mode' "$case_dir/output" +assert_contains 'launchctl load' "$case_dir/calls" +assert_not_contains 'ctrld start' "$case_dir/calls" + +case_dir=$(run_case upgrade-standard-failure 1 1 standard 1 1) +assert_contains 'ctrld start --intercept-mode off' "$case_dir/calls" +assert_contains 'ERROR: upgrade installed but managed InterceptMode could not be applied' "$case_dir/output" +assert_not_contains 'launchctl load' "$case_dir/calls" + printf 'PASS: pkg postinstall preserves legacy mode and applies standard/intercept-dns policy\n'