Skip to content

feat(cli): offer to save denied open-url origins on exit#1222

Open
caiocdcs wants to merge 2 commits into
always-further:mainfrom
caiocdcs:1187-save-denied-open-url-origins
Open

feat(cli): offer to save denied open-url origins on exit#1222
caiocdcs wants to merge 2 commits into
always-further:mainfrom
caiocdcs:1187-save-denied-open-url-origins

Conversation

@caiocdcs

Copy link
Copy Markdown
Contributor

Linked Issue

Closes #1187

Summary

Fold fixable open-url denials into the existing exit-time profile save prompt.

When a sandboxed child asks the supervisor to open a URL that the profile does
not allow (common in OAuth flows like gcloud auth login, gh auth login),
the denial was previously dropped on the floor. After this change, the exit-time
selector accumulates these denials and offers to:

  • add the denied origin (e.g. https://accounts.google.com) to
    open_urls.allow_origins, or
  • enable open_urls.allow_localhost for blocked localhost callbacks.

Details matching the issue:

  • Only the origin (scheme + host) is saved, never the full URL, so OAuth
    tokens in query strings are never persisted.
  • Non-fixable denials (file:///javascript:, oversized URL, parse error,
    browser-launch failure) are classified CLI-side and never produce a record,
    so they are never offered in the save prompt.
  • URL items in the selector support grant / skip only (no per-origin
    suppress list).
  • Both the macOS and Linux supervisor loops accumulate and surface these
    denials.

The library stays policy-free: UrlDenialReason (in crates/nono) carries only
the two fixable variants that are actually persisted; the richer non-fixable
classification lives on the CLI-side UrlDenial enum.

Test Plan

  • make clippy — clean (-D warnings -D clippy::unwrap_used)
  • make fmt-check — clean
  • cargo test -p nono -p nono-cli --bins --lib — nono 670 passed, nono-cli 1303 passed
  • New unit coverage:
    • test_url_denial_maps_to_record_for_save_prompt — a denied origin/blocked
      localhost maps to the correct UrlDenialRecord (origin-only payload);
      non-fixable denials map to no record.
    • build_url_patch_*, build_combined_patch_from_items_*,
      merge_profile_patch_* (origins dedup-append, localhost monotonic
      false->true).
    • test_profile_save_prompt_triggers_on_url_denial_with_zero_exit.

Note: test_open_url_helper_binary_succeeds_with_valid_supervisor fails in my
local checkout because the workspace path makes the supervisor socket path
exceed SUN_LEN. This is pre-existing (reproduces on a clean main) and
environment-specific, not introduced by this change.

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
  • Release note has been added to CHANGELOG.md if needed

@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

PR Review Summary

Size

Metric Value
Lines added +1091
Lines removed -259
Total changed 1350
Classification Large (> 300 lines)

Affected crates

  • crates/nono (core library) — careful review required. This is the security-critical sandbox primitive. A bug here bypasses OS-level isolation for every downstream user.
  • crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Contained

This PR touches: source code


Updated automatically on each push to this PR.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces URL denial tracking and validation to the supervisor loop, allowing users to save allowed origins and localhost configurations directly from the interactive profile save prompt. The feedback highlights two key improvements: first, ensuring that pressing the 'deny-all' key (d) in the interactive selector correctly sets URL items to Skip instead of leaving them as Grant to prevent accidental permission grants; second, deduplicating URL denials inline within record_url_denial to prevent buffer exhaustion from repeated requests to the same blocked URL.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread crates/nono-cli/src/profile_save_runtime.rs Outdated
Comment thread crates/nono-cli/src/exec_strategy.rs
@caiocdcs caiocdcs force-pushed the 1187-save-denied-open-url-origins branch from f5cc211 to e213faa Compare June 22, 2026 10:45
caiocdcs added 2 commits June 22, 2026 17:29
Fold fixable open-url denials into the exit-time profile save prompt.
After a supervised run where a URL open was denied because its origin is
not in open_urls.allow_origins, or because allow_localhost is false, the
selector now offers to grant the origin or enable localhost and persist
it to a user profile.

Only the origin (scheme + host) is saved, never the full URL, so OAuth
tokens in query strings are not persisted. Non-fixable denials (bad
scheme, oversized, parse errors, browser-launch failures) are classified
CLI-side and never produce a record, so they are never offered. Both the
macOS and Linux supervisor loops accumulate URL denials.

Signed-off-by: Caio Silva <caio@cdcs.dev>
- Factor the Linux supervisor loop's 4-tuple return into a
  SupervisorLoopResult type alias to satisfy clippy::type_complexity
  (only tripped on Linux, where the loop also returns URL records).
- Deny-all in the interactive selector now skips URL items so pressing
  Enter afterwards cannot grant the default Grant action.
- Deduplicate URL denial records inline so a child polling the same
  blocked URL cannot exhaust the record buffer.

Signed-off-by: Caio Silva <caio@cdcs.dev>
@caiocdcs caiocdcs force-pushed the 1187-save-denied-open-url-origins branch from e213faa to 4712e5a Compare June 22, 2026 16:53
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.

Offer to save denied open-url origins to the user profile on exit

1 participant