Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/agentic_cli/cli/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from agentic_cli.cli.workflow_controller import WorkflowController
from agentic_cli.config import BaseSettings
from agentic_cli.logging import Loggers, configure_logging
from agentic_cli.settings_persistence import PROJECT_SETTABLE_KEYS

if TYPE_CHECKING:
from agentic_cli.settings_persistence import SettingsSaveResult
Expand All @@ -33,6 +34,12 @@

logger = Loggers.cli()

# Synthetic dialog keys → the settings field their setter actually writes.
# "model" is not a field; update_setting() routes it via set_model() to
# default_model. A synthetic key without an entry here is treated as writing
# a field of the same name.
_UI_KEY_TARGET_FIELDS = {"model": "default_model"}


# === Slash Command Completer ===

Expand Down Expand Up @@ -196,6 +203,11 @@ def get_ui_setting_keys(self) -> list[str]:
Override to customize which settings appear in the UI.
Default: model, thinking_effort

Only project-scoped settings may appear: a key whose target field is
not in PROJECT_SETTABLE_KEYS is excluded by _build_ui_items (with a
warning), because a /settings edit to it would persist in the user
~/.{app}/settings.json and apply across all projects of the app.

Returns:
List of field names that should appear in the settings UI
"""
Expand All @@ -216,6 +228,15 @@ def _build_ui_items(self) -> list[Any]:
items: list[tuple[int, Any]] = []

for key in self.get_ui_setting_keys():
# Project-scope guard: the dialog may only expose settings the
# project file can persist (PROJECT_SETTABLE_KEYS). A user-scoped
# key would save to the user ~/.{app}/settings.json and leak the
# change across all projects of the app.
target_field = _UI_KEY_TARGET_FIELDS.get(key, key)
if target_field not in PROJECT_SETTABLE_KEYS:
logger.warning("user_scoped_setting_excluded_from_ui", key=key)
continue

# Handle special 'model' field with dynamic options
if key == "model":
available_models = list(self._settings.get_available_models())
Expand Down
93 changes: 93 additions & 0 deletions tests/cli/test_settings_ui_scope.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
"""Guard: the /settings dialog exposes only project-scoped (allowlisted) settings.

User-scoped keys (not in PROJECT_SETTABLE_KEYS) must never render in the
dialog, regardless of what a domain app returns from get_ui_setting_keys() —
otherwise a /settings edit would persist to the user ~/.{app}/settings.json
and apply across all projects.
"""

from __future__ import annotations

import pytest
import structlog

from agentic_cli.cli.app import BaseCLIApp

EXCLUDED_EVENT = "user_scoped_setting_excluded_from_ui"


def _make_app(settings, keys: list[str]) -> BaseCLIApp:
class _App(BaseCLIApp):
def get_ui_setting_keys(self) -> list[str]:
return keys

app = _App.__new__(_App)
app._settings = settings
return app


@pytest.fixture
def isolated_settings(tmp_path, monkeypatch):
"""Hermetic BaseSettings: temp cwd/HOME so no real config files load."""
home = tmp_path / "home"
home.mkdir()
monkeypatch.chdir(tmp_path)
monkeypatch.setenv("HOME", str(home))
from agentic_cli.config import BaseSettings

# A key so get_available_models() is non-empty and "model" can render
return BaseSettings(google_api_key="test-key")


def _item_keys(items) -> list[str]:
return [item.key for item in items]


class TestSettingsUiScopeGuard:
def test_user_scoped_key_is_excluded_with_warning(self, isolated_settings):
"""A non-allowlisted field never renders; the exclusion is logged."""
app = _make_app(isolated_settings, ["thinking_effort", "raw_llm_logging"])

with structlog.testing.capture_logs() as logs:
items = app._build_ui_items()

assert _item_keys(items) == ["thinking_effort"]
assert any(
e.get("event") == EXCLUDED_EVENT and e.get("key") == "raw_llm_logging"
for e in logs
)

def test_allowlisted_keys_render_without_warning(self, isolated_settings):
app = _make_app(isolated_settings, ["thinking_effort", "verbose_thinking"])

with structlog.testing.capture_logs() as logs:
items = app._build_ui_items()

assert set(_item_keys(items)) == {"thinking_effort", "verbose_thinking"}
assert not any(e.get("event") == EXCLUDED_EVENT for e in logs)

def test_model_synthetic_key_passes_guard(self, isolated_settings):
""""model" is not a field but writes default_model (allowlisted)."""
app = _make_app(isolated_settings, ["model"])

with structlog.testing.capture_logs() as logs:
items = app._build_ui_items()

assert _item_keys(items) == ["model"]
assert not any(e.get("event") == EXCLUDED_EVENT for e in logs)

def test_dangling_nonexistent_key_is_excluded_with_warning(
self, isolated_settings
):
"""A key for a removed field (e.g. airesearcher's log_activity) now
warns instead of being silently skipped."""
app = _make_app(isolated_settings, ["log_activity"])

with structlog.testing.capture_logs() as logs:
items = app._build_ui_items()

assert items == []
assert any(
e.get("event") == EXCLUDED_EVENT and e.get("key") == "log_activity"
for e in logs
)
Loading