Skip to content

feat(cli): add native dialog approval backend - #1775

Open
panga wants to merge 5 commits into
nolabs-ai:mainfrom
panga:feat/dialog-approval-backend
Open

feat(cli): add native dialog approval backend#1775
panga wants to merge 5 commits into
nolabs-ai:mainfrom
panga:feat/dialog-approval-backend

Conversation

@panga

@panga panga commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #1774

Summary

Adds a native OS-dialog approval backend (dialog) for supervised approvals.

The existing terminal backend is unusable inside terminal UIs that own the
controlling terminal (e.g. AI coding agents like Claude Code, and any raw-mode
TUI): stderr is piped so the is_terminal() check auto-denies, and even
with a tty the blocking /dev/tty read contends with the TUI's raw-mode
ownership, so the prompt is invisible/unanswerable. The dialog backend prompts
via a native modal delivered by the window/display server — a channel separate
from the controlling terminal — so approvals work even while a TUI owns the tty.

  • macOS: osascript modal dialog.
  • Linux: zenity / kdialog on desktop sessions.
  • Selected like any other backend via security.approval_backends /
    approval_defaults, and composable inside a chain.

It is a pure CLI-side addition alongside the existing terminal/webhook/
chain backends: it implements the library's ApprovalBackend trait without
any library change, so the library/CLI boundary is untouched.

Security posture (fail-secure, no injection):

  • Fail secure: when no GUI display is reachable (headless, SSH, container,
    CI) it denies immediately without spawning anything and never falls back to
    the terminal. macOS gates on an Aqua GUI session (launchctl managername) so
    SSH-into-a-Mac denies rather than hijacking the console user's screen; Linux
    requires $DISPLAY/$WAYLAND_DISPLAY.
  • No injection: every untrusted ApprovalRequest field is control/ANSI
    stripped (reusing sanitize_for_terminal) and length-bounded, then passed to
    the dialog tool as a single process argument — never interpolated into an
    AppleScript program or a shell string. The message always begins with a fixed
    non-- prefix so untrusted content can never be parsed as a CLI option; Linux
    markup is entity-escaped; dialog binaries are located by absolute path (never
    $PATH).
  • Fixed choices: Allow/Deny button labels are compile-time constants and
    Deny is the default; untrusted text never reaches a button.
  • Bounded wait: the dialog is force-terminated after a wall-clock deadline,
    resolving a walked-away user to Timeout (a denial) rather than blocking.

Changes

  • crates/nono-cli/src/dialog_approval.rs (new)DialogApproval backend.
  • crates/nono-cli/src/main.rs — register mod dialog_approval.
  • crates/nono-cli/src/command_policy.rsDialog variant, validation arm
    (rejects url/mode/backends), and validation tests.
  • crates/nono-cli/src/approval_runtime.rs — builder arm constructing the backend.
  • crates/nono-cli/data/nono-profile.schema.json — add dialog to the
    backend-kind enum.
  • docs/cli/features/supervisor.mdx — document the backend (and correct the
    stale "webhook planned" note).

Agent Disclosure

  • This PR was prepared by an AI agent (Claude Code), acting on behalf of the
    repository maintainer.
  • Files/sections consulted: AGENTS.md (library/CLI boundary, security
    considerations, contribution policy); crates/nono/src/supervisor/mod.rs
    (ApprovalBackend trait) and .../supervisor/types.rs (ApprovalRequest /
    ApprovalDecision); crates/nono-cli/src/terminal_approval.rs (reused
    sanitize_for_terminal, prompt patterns); crates/nono-cli/src/approval_runtime.rs
    (backend builder/registry patterns, mirrored from WebhookApproval);
    crates/nono-cli/src/command_policy.rs (config types and per-type validation).
  • The maintainer determined that no NEP is required for this change: it plugs
    into existing approval-backend extension points and does not move the
    library/CLI security boundary.
  • I confirm this contribution complies with the repository's coding and security
    requirements (see checklist below).

Test Plan

Screenshot 2026-09-03 at 10 00 26 AM
cargo build -p nono-cli
cargo clippy --workspace --all-targets --all-features -- -D warnings -D clippy::unwrap_used
cargo fmt --all -- --check
cargo test -p nono-cli

Checklist

  • An issue exists and is linked above
  • All commits are signed-off, using DCO
  • All new code follows the project's coding standards (CLAUDE.md) and is covered by tests
  • Public-facing changes are paired with documentation updates
  • If this PR introduces a major feature, capability, or security-relevant change, a corresponding NEP has been opened or accepted and is linked — maintainer waived the NEP requirement for this change (see Agent Disclosure).

Agent Compliance Check

  • I am not prohibited from contributing under this policy
  • An issue already exists
  • I disclosed that I am an agent in the issue discussion
  • I described my intent and approach in the issue discussion
  • I reviewed repository coding and security rules for the affected area
  • I provided required attribution for reused or adapted code (reused sanitize_for_terminal; mirrored WebhookApproval construction patterns)
  • I did not use forbidden patterns such as unwrap/expect
  • I used NonoError where required
  • I validated and canonicalized all relevant paths — N/A: no user-supplied paths are granted here; dialog binaries are fixed absolute-path constants (never $PATH)
  • This PR matches the approved or disclosed issue scope

Add a `dialog` approval backend that prompts via a native OS dialog
(macOS osascript, Linux zenity/kdialog) instead of the terminal. The
`terminal` backend auto-denies or hangs inside terminal UIs that own the
controlling terminal (e.g. AI coding agents); a native dialog is delivered
by the window/display server, a channel separate from the tty, so it works
in that environment.

It implements the library `ApprovalBackend` trait with no library change
and slots into the existing backend config/registry alongside terminal,
webhook, and chain. Selected via `security.approval_backends` /
`approval_defaults` and composable inside a chain.

Security:
- Fail secure: denies immediately with no subprocess when no GUI display
  is reachable (headless/SSH/container/CI); no terminal fallback. macOS
  gates on an Aqua session so SSH-into-a-Mac denies rather than hijacking
  the console screen; Linux requires DISPLAY/WAYLAND_DISPLAY.
- No injection: untrusted request fields are ANSI/control stripped and
  length-bounded, then passed as single process arguments (never
  interpolated into AppleScript or a shell). The message starts with a
  fixed non-dash prefix so it cannot be parsed as a CLI option; Linux
  markup is entity-escaped; binaries resolved by absolute path only.
- Fixed Allow/Deny labels with Deny default; bounded wall-clock timeout
  resolves an unattended prompt to Timeout (a denial).

Closes nolabs-ai#1774

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Leonardo Zanivan <leonardo.zanivan@gmail.com>
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +765
Lines removed -6
Total changed 771
Classification Large (> 300 lines)

Affected crates

  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Moderate

This PR touches: source code,configuration / policy files


Updated automatically on each push to this PR.

@nogent-nolabs-ai nogent-nolabs-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nogent code review

1 thread-safety test issue.

Automated code + security review. CI already covers clippy, rustfmt, tests, cargo-audit and commit-lint.

Comment thread crates/nono-cli/src/dialog_approval.rs
…n test

Replace the local OnceLock<Mutex<()>> in linux_denies_without_display with
crate::test_env::ENV_LOCK so all env-mutating tests in the process share one
serialization point. Wrap set_var/remove_var calls in unsafe blocks as required
by Rust's updated safety contract for environment mutation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Leonardo Zanivan <leonardo.zanivan@gmail.com>
The project bans direct set_var/remove_var via a custom clippy lint.
Use EnvVarGuard::set_all to register keys (capturing originals for
restore on drop), then call .remove() on both keys to simulate a
headless display environment for the linux_denies_without_display test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Leonardo Zanivan <leonardo.zanivan@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Leonardo Zanivan <leonardo.zanivan@gmail.com>
Instead of separate Command:/Args: rows, show the full invocation as one
Command: line. Lines wider than 80 chars are broken at word boundaries
with a hanging indent so the value columns stay aligned.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Leonardo Zanivan <leonardo.zanivan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

native dialog approval backend for supervised approvals

1 participant