Track entries by full tuple (pid/proto/addr/state); improve container lookup, sudo flow, SSH tunnels, and watch UI#6
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97ea33c26d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let scan_result = if self.is_elevated { | ||
| scanner::scan_elevated() |
There was a problem hiding this comment.
Switching refresh to scanner::scan_elevated() means macOS now silently falls back to unprivileged scan() when sudo -n credentials expire, but this path never clears is_elevated/is_root; the session can stay in a permanently "sudo OK" state even though privileged scans and sudo -n firewall actions no longer work. In that state users cannot re-enter a password from the UI because input handling only opens the sudo prompt when !is_root, so they are effectively stuck until restart.
Useful? React with 👍 / 👎.
| let _ = stdout.read_to_string(&mut out); | ||
| } | ||
| return if out.is_empty() { None } else { Some(out) }; | ||
| return Some(out); |
There was a problem hiding this comment.
Returning Some(out) for successful commands with empty stdout changes control flow in resolve_container_names: docker_resolve now returns Some(empty_map) when Docker is installed but has zero running containers, so or_else(|| podman_resolve(...)) is never reached. On hosts where Docker is present but workloads run under Podman, container names will no longer be resolved.
Useful? React with 👍 / 👎.
empty docker result
fix: reset expired sudo elevation and restore podman fallback after
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0de283d7ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Some((ip, _)) = self.confirm_block.take() { | ||
| let sudo_pw = self.session.sudo_password(); | ||
| match firewall::execute_block(ip, sudo_pw) { | ||
| match firewall::execute_block(ip, None) { |
There was a problem hiding this comment.
Calling firewall::execute_block(ip, None) forces the block action down the non-interactive sudo -n path, so it now fails whenever no sudo timestamp is already cached. This is a regression for non-root users, especially on Linux: pressing s no longer helps because scan_ports_with_sudo there does not invoke sudo (it just calls linux::scan()), so the app reports elevation but still cannot run iptables/pfctl from the block flow unless sudo was primed outside the app.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef0e9140d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub fn refresh(&mut self) -> Result<(), String> { | ||
| let scan_result = if let Some(password) = &self.sudo_password { | ||
| scanner::scan_with_sudo(password) | ||
| self.sync_elevation_state(scanner::has_elevated_access()); |
There was a problem hiding this comment.
Session::refresh now calls scanner::has_elevated_access() every cycle even when the user never requested elevation; on Linux/macOS that probe executes sudo -n true via Command::status() (see platform helpers), which inherits stderr. For non-root users without a cached sudo timestamp, sudo emits a password is required, so the app can spam terminal output every tick and disrupt the TUI. Please only probe when is_elevated is set (or silence stdio for the probe).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Line 26 in 7be206d
Drain SSH stderr for long-lived tunnels
The tunnel process is spawned with stderr(Stdio::piped()), but stderr is only consumed in the immediate-failure check; for successful startups there is no reader attached. If ssh emits ongoing stderr output at runtime (for example repeated forwarding errors), the pipe buffer can fill and block the child process, causing tunnel behavior to hang unexpectedly. Keep stderr redirected to null after startup validation or continuously drain it.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| docker_resolve(pids) | ||
| .or_else(|| podman_resolve(pids)) | ||
| .unwrap_or_default() | ||
| select_runtime_names(docker_resolve(pids), podman_resolve(pids)) |
There was a problem hiding this comment.
resolve_container_names now calls select_runtime_names(docker_resolve(pids), podman_resolve(pids)), which eagerly evaluates both resolvers, so Podman CLI calls run every refresh even when Docker already returned container names. This is a regression from the prior lazy or_else behavior and can noticeably slow/lag refresh cycles on hosts where Podman commands are slow (including up to the timeout path), despite Docker data being sufficient.
Useful? React with 👍 / 👎.
…t, model health Auto-reconnect (#1,#2,#3,#8): - Backoff now actually grows for unreachable hosts. The reset was tied to a 'successful' 150ms spawn, which an unreachable host passes (it blocks on TCP timeout), so the delay never increased. Growth now happens on every attempt and is reset only after a tunnel stays Alive for STABLE_THRESHOLD (30s), via refresh_health. - reconnect uses a new non-blocking restart_async() (no 150ms sleep in the render thread); failures are caught on the next tick. Manual restart() stays blocking for immediate feedback. - After MAX_RETRIES attempts without recovery a tunnel is marked permanently failed (auto_reconnect=false); drop_failed() now prunes only those, so 'save' no longer discards a tunnel that's mid-reconnect. auto_reconnect is now a meaningful flag. Listener health in the model (#4,#5): - New TunnelStatus::Unhealthy (process alive, local port not listening). Decision extracted to a pure, unit-tested decide_status(); a HEALTH_GRACE window and a scan_usable flag suppress false 'no listener' right after start and while auto-refresh is paused. cleanup() now takes the listening port set; the view no longer reaches into the scan. Cleanups: - command_string() shell-quotes args via shlex (#6). - uptime column reuses prt-core format_duration (#7). - private from_parts() constructor removes spawn duplication (#9). - cross-reference comments between PROXY_PORTS and known_ports (#10). New unit tests: decide_status, next_backoff, command_string quoting.
1 participant
Motivation
Description
core::scannerfrom(local_port, pid)to(pid, protocol, local_addr, remote_addr, state)and addEntryKey,entry_key, andtracked_entry_keyhelpers, updatingdiff_entriesand related tests accordingly.Hashderives forProtocolandConnectionStateinmodel.rsto support use in hash keys.container.rsby building aHashSetof target PIDs for O(1) checks and stop treating an empty command stdout as a failure inrun_with_timeoutwhen the process exits successfully.Sessionand switch refresh/elevation logic to useis_elevatedwithscanner::scan_elevated, and stop exposingsudo_passwordfromSessionandApp.SshTunnel::newinforward.rsto detect and surface immediatesshfailures by waiting briefly and reading stderr before returning a successfully spawned tunnel.watch.rsto include a human-friendly "since" duration for UP/DOWN state and addformat_sinceplus tests for the new behavior.README.mdwith new watch examples, a new "High-ROI Use Cases" section, and a small dev instruction tweak to remove the hardcoded test count.Testing
cargo test --workspace, which executed the unit tests inprt-coreandprtincluding the newdiff_same_pid_port_but_different_remote_is_newandformat_since_secondstests, and all tests passed.Codex Task