diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index f463f5a..67306d9 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -40,7 +40,9 @@ "source=${localEnv:HOME}/.config/gh,target=/home/vscode/.config/gh,type=bind,consistency=cached", "source=${localEnv:HOME}/.claude,target=/home/vscode/.claude,type=bind,consistency=cached", "source=${localEnv:HOME}/.aws,target=/home/vscode/.aws,type=bind,consistency=cached", - "source=${localEnv:HOME}/.ssh,target=/home/vscode/.ssh,type=bind,consistency=cached" + "source=${localEnv:HOME}/.ssh,target=/home/vscode/.ssh,type=bind,consistency=cached", + "source=${localWorkspaceFolder}/../site,target=/workspaces/site,type=bind,consistency=cached", + "source=${localWorkspaceFolder}/../remo-broker,target=/workspaces/remo-broker,type=bind,consistency=cached" ], "postCreateCommand": "python3 --version && node --version" } diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 3841dd1..9cdb7ff 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -7,6 +7,10 @@ updates: interval: "weekly" day: "monday" open-pull-requests-limit: 5 + # Supply-chain cooldown: hold newly published versions for 7 days so a + # compromised release can be detected/yanked before it's proposed. + cooldown: + default-days: 7 groups: # One PR per week with all minor/patch production deps bumped together. production-minor-patch: @@ -29,8 +33,22 @@ updates: interval: "weekly" day: "monday" open-pull-requests-limit: 5 + cooldown: + default-days: 7 groups: actions: update-types: - "minor" - "patch" + + # Docker base images (Dockerfile + notifier/Dockerfile) + - package-ecosystem: "docker" + directories: + - "/" + - "/notifier" + schedule: + interval: "weekly" + day: "monday" + open-pull-requests-limit: 5 + cooldown: + default-days: 7 diff --git a/.github/workflows/smoke-test.yml b/.github/workflows/smoke-test.yml index 7d07250..c3188a4 100644 --- a/.github/workflows/smoke-test.yml +++ b/.github/workflows/smoke-test.yml @@ -803,6 +803,51 @@ jobs: $SSH_CMD "pkill -f 'python3 -m http.server 9090'" 2>/dev/null || true $SSH_CMD "pkill -f 'python3 -m http.server 3000'" 2>/dev/null || true + - name: Test remo shell -p / --exec / --detach (project-launch passthrough) + run: | + SSH_CMD="ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \ + -o ConnectTimeout=30 -o BatchMode=yes -i ~/.ssh/id_rsa remo@$HOST" + + # Plant a project dir without a .devcontainer so we exercise the + # host-side branches of project-launch (no docker dance needed in CI). + $SSH_CMD "mkdir -p ~/projects/smoke-proj && \ + echo 'placeholder' > ~/projects/smoke-proj/README" + + # --- Foreground --exec on a no-devcontainer project (Case 4) --- + OUT=$(uv run remo shell -p smoke-proj --exec 'echo hello-$REMO_PROJECT-$REMO_INSTANCE && pwd' 2>&1) + echo "$OUT" + echo "$OUT" | grep -q "hello-smoke-proj-" || { + echo "FAIL: --exec did not run / env vars missing"; exit 1; } + echo "$OUT" | grep -q "/projects/smoke-proj" || { + echo "FAIL: --exec did not cd into project dir"; exit 1; } + echo "remo shell -p --exec (foreground): PASS" + + # --- Detached --exec writes log file and returns immediately --- + DETACH_OUT=$(uv run remo shell -p smoke-proj --detach \ + --exec 'sleep 1 && echo detached-marker-$REMO_PROJECT' 2>&1) + echo "$DETACH_OUT" + echo "$DETACH_OUT" | grep -q "Launched detached" || { + echo "FAIL: detach didn't print launched message"; exit 1; } + # Give the background command a moment to finish + sleep 3 + LOG=$($SSH_CMD "cat ~/.local/state/remo/smoke-proj.log") + echo "log contents:"; echo "$LOG" + echo "$LOG" | grep -q "detached-marker-smoke-proj" || { + echo "FAIL: detached command did not run / log missing marker"; exit 1; } + echo "remo shell -p --detach --exec: PASS" + + # --- Validation: --exec without -p exits 2 --- + if uv run remo shell --exec 'true' 2>/dev/null; then + echo "FAIL: --exec without -p should have errored"; exit 1 + fi + echo "validation: --exec without -p errors as expected" + + # --- Validation: --detach without --exec exits 2 --- + if uv run remo shell -p smoke-proj --detach 2>/dev/null; then + echo "FAIL: --detach without --exec should have errored"; exit 1 + fi + echo "validation: --detach without --exec errors as expected" + - name: Teardown if: always() run: | diff --git a/.gitignore b/.gitignore index 20b5bb9..3c06c18 100644 --- a/.gitignore +++ b/.gitignore @@ -31,6 +31,12 @@ build/ *.egg-info/ *.egg +# Test / coverage artifacts +.coverage +.coverage.* +.pytest_cache/ +htmlcov/ + # Python virtual environment venv/ .venv/ @@ -45,3 +51,4 @@ env/ .maverick/* !.maverick/checkpoints/ .claude/settings.local.json +.DS_Store diff --git a/CLAUDE.md b/CLAUDE.md index 4292696..00c035f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,6 +12,8 @@ See `.specify/memory/constitution.md` for project principles and non-negotiable - Python 3.11+ + Click (CLI framework), InquirerPy (interactive picker), boto3 (AWS, optional), hcloud (Hetzner, optional) (003-python-cli-rewrite) - Flat file (`~/.config/remo/known_hosts`, colon-delimited) (003-python-cli-rewrite) - Cross-provider snapshot model (`models/snapshot.py`) + shared helpers in `core/snapshot.py` (name generator, validator, table formatter, destroy-time cleanup hook). No new runtime deps. (005-provider-snapshots) +- Python 3.11+ (package `requires-python = ">=3.11"`); service container runs Python 3.13-slim + Service (new `[notifier]` extra): FastAPI ≥0.115, uvicorn[standard] ≥0.32, pydantic ≥2.9, python-telegram-bot ≥21.6, structlog ≥24.4, tomli (py<3.11 only). CLI side: Click ≥8.1 (existing), no new laptop runtime deps. Build: hatchling (existing), uv (in-container). Ansible: new `community.docker` collection. (007-notifier-sidecar) +- None. All approval state is in-memory in a `PendingApprovals` registry; never persisted (FR-009). (007-notifier-sidecar) - Ansible 2.14+ / YAML + `ansible.builtin`, `community.general` (for zypper module) (001-bootstrap-incus-host) @@ -132,9 +134,9 @@ Provider SDKs (boto3, hcloud) are lazy-imported with clear error messages if mis - Ansible 2.14+ / YAML: Follow standard conventions plus Constitution principles ## Recent Changes +- 007-notifier-sidecar: Added Python 3.11+ (package `requires-python = ">=3.11"`); service container runs Python 3.13-slim + Service (new `[notifier]` extra): FastAPI ≥0.115, uvicorn[standard] ≥0.32, pydantic ≥2.9, python-telegram-bot ≥21.6, structlog ≥24.4, tomli (py<3.11 only). CLI side: Click ≥8.1 (existing), no new laptop runtime deps. Build: hatchling (existing), uv (in-container). Ansible: new `community.docker` collection. - 005-provider-snapshots: Added cross-provider snapshot CLI (`remo

snapshot {create,list,restore,delete}`) + destroy-time cleanup hook across Incus / Proxmox / AWS / Hetzner. - 003-python-cli-rewrite: Added Python 3.11+ + Click (CLI framework), InquirerPy (interactive picker), boto3 (AWS, optional), hcloud (Hetzner, optional) -- 002-incus-container-support: Added Ansible 2.14+ / YAML + `ansible.builtin`, `community.general` (existing), Incus CLI (local) diff --git a/README.md b/README.md index dfa1504..c234617 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,42 @@ The `fzf`-powered menu shows your projects from `~/projects`: - **c**: Clone a new repository - **x**: Exit to shell +### Jump Straight to a Project + +Skip the menu and land directly in a project (devcontainer auto-launches): + +```bash +remo shell -p my-app +``` + +Run a one-shot command inside the project's devcontainer instead of opening +a shell — quote the command as a single string: + +```bash +remo shell -p my-app --exec 'pytest -x' +remo shell -p my-app --exec 'claude --remote-control' +``` + +Fire-and-forget — kick off a command on the remote and exit SSH immediately: + +```bash +remo shell -p my-app --detach --exec 'claude remote-control --name remo-rc' +remo shell -p my-app --detach --exec './long-build.sh' +``` + +Detached output is captured to `~/.local/state/remo/.log` on the +remote, so you can tail it later (`remo shell -p my-app --exec 'tail -f +~/.local/state/remo/my-app.log'`). The command's environment gets +`REMO_INSTANCE` and `REMO_PROJECT` exported automatically — handy for +deterministic naming, e.g.: + +```bash +remo shell -p my-app --detach --exec \ + 'claude remote-control --name "remo-$REMO_INSTANCE-$REMO_PROJECT"' +``` + +Then on your phone, open claude.ai/code and pick the session by name. + ### Port Forwarding Forward remote ports to your local machine during SSH sessions: @@ -169,12 +205,66 @@ The `destroy` command on each provider checks for existing snapshots first and offers to clean them up. Decline and the snapshots remain (you'll be warned they continue to incur storage costs on AWS/Hetzner). +## Notifier + +The **notifier** is a small approval bridge that runs as a hardened container on +a remo host. When an agentsh-secured devcontainer needs human sign-off for an +operation, it POSTs an approval request to the notifier, which messages you on +Telegram with **Approve** / **Deny** buttons and returns your decision. It fails +secure: a timeout, a shutdown, or a lost connection all mean *deny*. The wire +protocol is documented in +[`src/remo_cli/notifier/docs/wire-protocol.md`](src/remo_cli/notifier/docs/wire-protocol.md). + +The notifier's runtime dependencies (FastAPI, python-telegram-bot, …) live in an +optional `notifier` extra and are installed only inside the container — a normal +`remo` install does not pull them in. + +### Notifier setup + +1. **Create a Telegram bot**: message [`@BotFather`](https://t.me/BotFather), run + `/newbot`, follow the prompts, and save the bot token. +2. **Find your chat id**: message `@userinfobot` from your own account; it + replies with your numeric user id — that's your `authorized_chat_id`. +3. **Message your new bot once** (any message) so it can DM you. +4. **Export credentials** on the machine where `remo` runs: + ```bash + export REMO_NOTIFIER_TELEGRAM_BOT_TOKEN="12345:ABC...your-token" + export REMO_NOTIFIER_TELEGRAM_CHAT_ID="987654321" + ``` +5. **Deploy**: `remo notifier deploy ` (omit `` to fuzzy-pick). +6. **Verify**: `remo notifier test ` — you should get a Telegram message + within ~2 s; tapping Approve or Deny returns the decision to the CLI. + +### Notifier commands + +```bash +remo notifier deploy [--rebuild] # apply the role; --rebuild forces a fresh image +remo notifier status # GET /v1/health +remo notifier logs [-f] [-n N] # journalctl -u remo-notifier.service +remo notifier test # push a test approval, print the decision +remo notifier restart # systemctl restart remo-notifier.service +``` + +### "Always" — standing grants + +Approval messages offer **✅ Approve · ⏩ Always… · ❌ Deny**. Tapping **Always…** +lets you auto-approve a *class* of operation (e.g. `git push *` in this project) +so matching requests are approved instantly without pinging you again. Grants are +held in memory only (cleared on restart → you're asked again — fail-closed), +expire after a default 8h, and default to the narrowest scope. Manage them from +Telegram: `/rules` (list + revoke), `/revoke `, `/pause` and `/resume`. +Auto-approvals are logged and summarized in a periodic digest. Tune via the +`[grants]` config block (see `src/remo_cli/notifier/docs/config-schema.md`). + ## CLI Reference ```bash # Connect to environment remo shell # Auto-connect (or picker if multiple) remo shell my-env # Connect to a specific environment +remo shell -p my-app # Skip the menu, jump to ~/projects/my-app +remo shell -p my-app --exec 'pytest -x' # Run command in devcontainer +remo shell -p my-app --detach --exec 'claude remote-control --name rc' # Fire and exit remo shell -L 8080 # Shell + forward remote :8080 to local :8080 remo shell -L 9000:8080 # Shell + forward remote :8080 to local :9000 remo shell -L 8080 -L 3000 # Shell + forward multiple ports diff --git a/ansible/group_vars/all.yml b/ansible/group_vars/all.yml index 458d612..636886c 100644 --- a/ansible/group_vars/all.yml +++ b/ansible/group_vars/all.yml @@ -27,3 +27,12 @@ zellij_session_name: "main" # Session name for SSH auto-attach # Node.js Configuration nodejs_version: "24" # LTS version + +# ============================================================================ +# Remo Notifier (spec 007) +# ============================================================================ +# Notifier sidecar that delivers agentsh approval requests to Telegram and +# returns the human's decision. Deployed by the remo_notifier role. + +remo_notifier_telegram_bot_token: "{{ lookup('env', 'REMO_NOTIFIER_TELEGRAM_BOT_TOKEN') }}" +remo_notifier_telegram_chat_id: "{{ lookup('env', 'REMO_NOTIFIER_TELEGRAM_CHAT_ID') }}" diff --git a/ansible/notifier_deploy.yml b/ansible/notifier_deploy.yml new file mode 100644 index 0000000..0858602 --- /dev/null +++ b/ansible/notifier_deploy.yml @@ -0,0 +1,17 @@ +--- +# Deploy the remo notifier to an existing, provisioned host. +# +# Usage (via the CLI): +# remo notifier deploy +# +# Direct usage: +# ansible-playbook notifier_deploy.yml -i "," -e "ansible_user=remo" \ +# -e "remo_notifier_build_context_local=/path/to/build/context" + +- name: Deploy remo notifier + hosts: all + become: true + gather_facts: true + + roles: + - role: remo_notifier diff --git a/ansible/requirements.yml b/ansible/requirements.yml index 0775ce7..7f162d7 100644 --- a/ansible/requirements.yml +++ b/ansible/requirements.yml @@ -12,3 +12,5 @@ collections: version: ">=11.0.0" - name: community.crypto version: ">=3.1.0" + - name: community.docker + version: ">=4.0.0" diff --git a/ansible/roles/remo_notifier/defaults/main.yml b/ansible/roles/remo_notifier/defaults/main.yml new file mode 100644 index 0000000..37262d3 --- /dev/null +++ b/ansible/roles/remo_notifier/defaults/main.yml @@ -0,0 +1,38 @@ +--- +# Defaults for the remo_notifier role. Override in group_vars/all.yml. + +remo_notifier_version: "0.1.0" +remo_notifier_image: "remo-notifier:{{ remo_notifier_version }}" +remo_notifier_listen_port: 18181 +remo_notifier_bind_address: "172.17.0.1" # Docker bridge address +remo_notifier_config_dir: "/etc/notifier" +remo_notifier_secrets_dir: "/etc/notifier/secrets" +remo_notifier_instance_id: "{{ inventory_hostname }}" +remo_notifier_default_timeout_seconds: 300 +remo_notifier_max_timeout_seconds: 1800 +remo_notifier_max_pending_approvals: 50 +remo_notifier_log_level: "info" +remo_notifier_message_parse_mode: "MarkdownV2" + +# Standing grants ("Always" auto-approval) — Addendum 001 +remo_notifier_grants_enabled: true +remo_notifier_grants_default_ttl_seconds: 28800 # 8h; every grant expires +remo_notifier_grants_max: 100 +remo_notifier_grants_allow_global_scope: true +remo_notifier_grants_digest_interval_seconds: 3600 # 0 disables the digest + +remo_notifier_telegram_bot_token: "{{ lookup('env', 'REMO_NOTIFIER_TELEGRAM_BOT_TOKEN') }}" +remo_notifier_telegram_chat_id: "{{ lookup('env', 'REMO_NOTIFIER_TELEGRAM_CHAT_ID') }}" + +# v1 builds the image on the host from a bundled context; future: pull from a +# registry by setting this to false. +remo_notifier_build_from_source: true +remo_notifier_source_dir: "/opt/remo-notifier-build" + +# Absolute path to the build context bundled in the wheel +# (remo_cli/notifier_build/). The CLI resolves and passes this; when running the +# role directly, point it at a repo checkout root. +remo_notifier_build_context_local: "" + +# Force a rebuild even if the image already exists. +remo_notifier_force_rebuild: false diff --git a/ansible/roles/remo_notifier/handlers/main.yml b/ansible/roles/remo_notifier/handlers/main.yml new file mode 100644 index 0000000..c3141cf --- /dev/null +++ b/ansible/roles/remo_notifier/handlers/main.yml @@ -0,0 +1,11 @@ +--- +- name: Reload systemd + ansible.builtin.systemd: + daemon_reload: true + listen: "Restart remo-notifier" + +- name: Restart remo-notifier + ansible.builtin.systemd: + name: remo-notifier.service + state: restarted + listen: "Restart remo-notifier" diff --git a/ansible/roles/remo_notifier/meta/main.yml b/ansible/roles/remo_notifier/meta/main.yml new file mode 100644 index 0000000..ddfe2ea --- /dev/null +++ b/ansible/roles/remo_notifier/meta/main.yml @@ -0,0 +1,5 @@ +--- +# The notifier runs as a Docker container, so it depends on the docker role +# being applied first (installs the Docker runtime + community.docker prereqs). +dependencies: + - role: docker diff --git a/ansible/roles/remo_notifier/tasks/main.yml b/ansible/roles/remo_notifier/tasks/main.yml new file mode 100644 index 0000000..0bdf480 --- /dev/null +++ b/ansible/roles/remo_notifier/tasks/main.yml @@ -0,0 +1,109 @@ +--- +# Deploy the remo notifier as a hardened Docker container managed by systemd. +# Registered-variable access uses | default() throughout (Constitution I). + +- name: Pre-flight — Telegram credentials are present + ansible.builtin.assert: + that: + - remo_notifier_telegram_bot_token | default('') | length > 0 + - remo_notifier_telegram_chat_id | default('') | string | length > 0 + fail_msg: >- + Missing Telegram credentials. Set REMO_NOTIFIER_TELEGRAM_BOT_TOKEN and + REMO_NOTIFIER_TELEGRAM_CHAT_ID in your environment before deploying the + notifier (see the README "Notifier setup" section). + success_msg: "Telegram credentials present." + +- name: Create notifier config and secrets directories + ansible.builtin.file: + path: "{{ item }}" + state: directory + owner: root + group: root + mode: "0700" + loop: + - "{{ remo_notifier_config_dir }}" + - "{{ remo_notifier_secrets_dir }}" + +- name: Render notifier config + ansible.builtin.template: + src: notifier.toml.j2 + dest: "{{ remo_notifier_config_dir }}/notifier.toml" + owner: root + group: root + mode: "0640" + notify: "Restart remo-notifier" + +- name: Write Telegram bot token secret + ansible.builtin.copy: + content: "{{ remo_notifier_telegram_bot_token }}" + dest: "{{ remo_notifier_secrets_dir }}/telegram_bot_token" + owner: root + group: root + mode: "0400" + no_log: true + notify: "Restart remo-notifier" + +- name: Stage the image build context on the host + ansible.builtin.copy: + src: "{{ remo_notifier_build_context_local }}/" + dest: "{{ remo_notifier_source_dir }}/" + mode: "0644" + directory_mode: "0755" + when: + - remo_notifier_build_from_source | default(true) | bool + - remo_notifier_build_context_local | default('') | length > 0 + +- name: Build the notifier image on the host + community.docker.docker_image: + name: "{{ remo_notifier_image }}" + source: build + force_source: "{{ remo_notifier_force_rebuild | default(false) | bool }}" + build: + path: "{{ remo_notifier_source_dir }}" + dockerfile: notifier/Dockerfile + when: remo_notifier_build_from_source | default(true) | bool + notify: "Restart remo-notifier" + +- name: Pull the notifier image + community.docker.docker_image: + name: "{{ remo_notifier_image }}" + source: pull + when: not (remo_notifier_build_from_source | default(true) | bool) + notify: "Restart remo-notifier" + +- name: Install systemd unit + ansible.builtin.template: + src: remo-notifier.service.j2 + dest: /etc/systemd/system/remo-notifier.service + owner: root + group: root + mode: "0644" + notify: "Restart remo-notifier" + +- name: Enable and start the notifier service + ansible.builtin.systemd: + name: remo-notifier.service + enabled: true + state: started + daemon_reload: true + +- name: Apply any pending restarts before health-checking + ansible.builtin.meta: flush_handlers + +- name: Wait for the notifier health endpoint + ansible.builtin.uri: + url: "http://{{ remo_notifier_bind_address }}:{{ remo_notifier_listen_port }}/v1/health" + status_code: 200 + register: notifier_health + retries: 10 + delay: 2 + until: notifier_health.status | default(0) == 200 + changed_when: false + +- name: Report notifier status + ansible.builtin.debug: + msg: >- + remo-notifier is healthy on + {{ remo_notifier_bind_address }}:{{ remo_notifier_listen_port }} + (pending approvals: + {{ notifier_health.json.pending_approvals | default('unknown') }}). diff --git a/ansible/roles/remo_notifier/templates/notifier.toml.j2 b/ansible/roles/remo_notifier/templates/notifier.toml.j2 new file mode 100644 index 0000000..6cf1dac --- /dev/null +++ b/ansible/roles/remo_notifier/templates/notifier.toml.j2 @@ -0,0 +1,30 @@ +{{ ansible_managed | comment }} +# Rendered by the remo_notifier Ansible role. Do not edit by hand. + +[server] +listen_host = "0.0.0.0" +listen_port = 18181 +log_level = "{{ remo_notifier_log_level }}" + +[approval] +default_timeout_seconds = {{ remo_notifier_default_timeout_seconds | int }} +max_timeout_seconds = {{ remo_notifier_max_timeout_seconds | int }} +max_pending_approvals = {{ remo_notifier_max_pending_approvals | int }} + +[grants] +enabled = {{ remo_notifier_grants_enabled | bool | lower }} +default_ttl_seconds = {{ remo_notifier_grants_default_ttl_seconds | int }} +max_grants = {{ remo_notifier_grants_max | int }} +allow_global_scope = {{ remo_notifier_grants_allow_global_scope | bool | lower }} +digest_interval_seconds = {{ remo_notifier_grants_digest_interval_seconds | int }} + +[transport] +type = "telegram" + +[transport.telegram] +bot_token_file = "/run/secrets/telegram_bot_token" +authorized_chat_id = {{ remo_notifier_telegram_chat_id | int }} +message_parse_mode = "{{ remo_notifier_message_parse_mode }}" + +[instance] +id = "{{ remo_notifier_instance_id }}" diff --git a/ansible/roles/remo_notifier/templates/remo-notifier.service.j2 b/ansible/roles/remo_notifier/templates/remo-notifier.service.j2 new file mode 100644 index 0000000..3194df0 --- /dev/null +++ b/ansible/roles/remo_notifier/templates/remo-notifier.service.j2 @@ -0,0 +1,26 @@ +{{ ansible_managed | comment }} +[Unit] +Description=Remo Notifier (agentsh approval bridge) +After=docker.service +Requires=docker.service + +[Service] +# Clean up any stale container before starting (idempotent restarts). +ExecStartPre=-/usr/bin/docker rm -f remo-notifier +ExecStart=/usr/bin/docker run --rm \ + --name remo-notifier \ + --network bridge \ + -p {{ remo_notifier_bind_address }}:{{ remo_notifier_listen_port }}:18181 \ + -v {{ remo_notifier_config_dir }}/notifier.toml:/etc/notifier/notifier.toml:ro \ + -v {{ remo_notifier_secrets_dir }}/telegram_bot_token:/run/secrets/telegram_bot_token:ro \ + --read-only \ + --tmpfs /tmp:rw,size=64m \ + --cap-drop ALL \ + --user 65532:65532 \ + {{ remo_notifier_image }} +ExecStop=/usr/bin/docker stop remo-notifier +Restart=always +RestartSec=5 + +[Install] +WantedBy=multi-user.target diff --git a/ansible/roles/user_setup/tasks/main.yml b/ansible/roles/user_setup/tasks/main.yml index 74d3323..ce0fedb 100644 --- a/ansible/roles/user_setup/tasks/main.yml +++ b/ansible/roles/user_setup/tasks/main.yml @@ -330,13 +330,29 @@ echo "Entering devcontainer (exit to return to host shell)..." echo "" + # If REMO_DEVCONTAINER_CMD is set (project-launch passthrough), + # run that command inside the devcontainer instead of dropping + # into the user's shell. Wrapped in `bash -lc` so the user's + # command gets variable expansion, pipes, &&, and PATH from + # the devcontainer's login profile. + if [[ -n "$REMO_DEVCONTAINER_CMD" ]]; then + _bash_flags="-lc" + _exec_cmd="$REMO_DEVCONTAINER_CMD" + else + _bash_flags="-c" + _exec_cmd='if command -v zsh &> /dev/null; then exec zsh; else exec bash; fi' + fi + # Run devcontainer shell (not exec, so we can stop container after) - if ! devcontainer exec --workspace-folder "$_project_dir" /bin/bash -c \ - 'if command -v zsh &> /dev/null; then exec zsh; else exec bash; fi'; then + if ! devcontainer exec --workspace-folder "$_project_dir" \ + env REMO_INSTANCE="${REMO_INSTANCE:-$(hostname)}" \ + REMO_PROJECT="$ZELLIJ_SESSION_NAME" \ + /bin/bash $_bash_flags "$_exec_cmd"; then echo "" echo "devcontainer exec failed. [Press any key to continue]" read -n 1 -s -r fi + unset _exec_cmd _bash_flags # After exiting devcontainer, stop the container to free resources echo "" @@ -393,6 +409,14 @@ group: "{{ remo_user }}" mode: '0755' +- name: Install project-launch script + ansible.builtin.template: + src: project-launch.sh.j2 + dest: "/home/{{ remo_user }}/.local/bin/project-launch" + owner: "{{ remo_user }}" + group: "{{ remo_user }}" + mode: '0755' + - name: Verify remo user exists ansible.builtin.command: id {{ remo_user }} register: user_setup_user_info diff --git a/ansible/roles/user_setup/templates/project-launch.sh.j2 b/ansible/roles/user_setup/templates/project-launch.sh.j2 new file mode 100644 index 0000000..8741023 --- /dev/null +++ b/ansible/roles/user_setup/templates/project-launch.sh.j2 @@ -0,0 +1,147 @@ +#!/bin/bash +# project-launch - Launch a project session non-interactively. +# Managed by Ansible - do not edit manually +# +# Usage: +# project-launch --project NAME [--detach] [--exec SHELL_COMMAND] +# +# SHELL_COMMAND is a single string run via `bash -lc`, so variable +# expansion, pipes, `&&`, etc. all work as written. +# +# Modes: +# Interactive (default): +# Acts like "pick NAME in project-menu". Drops into the project's +# zellij session; the existing .bashrc DEVCONTAINER block brings +# up the devcontainer and execs the user's shell (or SHELL_COMMAND +# when --exec is supplied — passed via REMO_DEVCONTAINER_CMD). +# +# Detached (--detach): +# Requires --exec. Brings up the devcontainer (or runs on the +# host project dir if no .devcontainer), launches the command in +# the background via nohup+setsid + `bash -lc`, captures +# stdout/stderr to ~/.local/state/remo/.log, and +# returns immediately. +# +# Environment passed to SHELL_COMMAND (both modes): +# REMO_INSTANCE - hostname of the remo instance +# REMO_PROJECT - the project name + +set -e + +PROJECTS_DIR="{{ dev_workspace_dir }}" + +PROJECT="" +DETACH=false +EXEC_CMD="" + +while [[ $# -gt 0 ]]; do + case "$1" in + --project) + PROJECT="$2" + shift 2 + ;; + --detach) + DETACH=true + shift + ;; + --exec) + EXEC_CMD="$2" + shift 2 + ;; + -h|--help) + sed -n '2,25p' "$0" | sed 's/^# \{0,1\}//' + exit 0 + ;; + *) + echo "project-launch: unknown argument: $1" >&2 + exit 2 + ;; + esac +done + +if [[ -z "$PROJECT" ]]; then + echo "project-launch: --project NAME is required" >&2 + exit 2 +fi + +PROJECT_DIR="$PROJECTS_DIR/$PROJECT" +if [[ ! -d "$PROJECT_DIR" ]]; then + echo "project-launch: project not found: $PROJECT_DIR" >&2 + exit 1 +fi + +HAS_DC=false +if [[ -d "$PROJECT_DIR/.devcontainer" ]] || [[ -f "$PROJECT_DIR/.devcontainer.json" ]]; then + HAS_DC=true +fi + +REMO_INSTANCE="$(hostname)" +export REMO_INSTANCE +export REMO_PROJECT="$PROJECT" + +# --------------------------------------------------------------------------- +# Detached mode: no zellij, no interactive shell. Run EXEC_CMD in background. +# --------------------------------------------------------------------------- +if $DETACH; then + if [[ -z "$EXEC_CMD" ]]; then + echo "project-launch: --detach requires --exec SHELL_COMMAND" >&2 + exit 2 + fi + + LOG_DIR="$HOME/.local/state/remo" + mkdir -p "$LOG_DIR" + LOG_FILE="$LOG_DIR/$PROJECT.log" + + cd "$PROJECT_DIR" + + if $HAS_DC; then + echo "Bringing up devcontainer for $PROJECT..." + devcontainer up --workspace-folder "$PROJECT_DIR" >/dev/null + printf -- '----- %s detached launch: %s -----\n' "$(date -Iseconds)" "$EXEC_CMD" >>"$LOG_FILE" + nohup setsid devcontainer exec --workspace-folder "$PROJECT_DIR" \ + env REMO_INSTANCE="$REMO_INSTANCE" REMO_PROJECT="$PROJECT" \ + bash -lc "$EXEC_CMD" >>"$LOG_FILE" 2>&1 >"$LOG_FILE" + nohup setsid env REMO_INSTANCE="$REMO_INSTANCE" REMO_PROJECT="$PROJECT" \ + bash -lc "$EXEC_CMD" >>"$LOG_FILE" 2>&1 /dev/null | sed 's/\x1b\[[0-9;]*m//g' | grep -q "^$PROJECT.*EXITED"; then + zellij delete-session "$PROJECT" 2>/dev/null || true +fi + +exec zellij attach --create "$PROJECT" diff --git a/ansible/tasks/configure_dev_tools.yml b/ansible/tasks/configure_dev_tools.yml index b9aad15..22e3d53 100644 --- a/ansible/tasks/configure_dev_tools.yml +++ b/ansible/tasks/configure_dev_tools.yml @@ -76,6 +76,14 @@ name: zellij when: configure_zellij | default(true) | bool +# Opt-in: the notifier requires Telegram credentials and is normally stood up +# via `remo notifier deploy `. It is NOT part of the default dev-tools +# configure flow (deploying it without creds would fail the preflight assert). +- name: Deploy Remo Notifier + ansible.builtin.include_role: + name: remo_notifier + when: configure_remo_notifier | default(false) | bool + - name: Write remo version marker ansible.builtin.copy: content: "{{ remo_version | default('') }}" diff --git a/docs/remo-fnox-spec.md b/docs/remo-fnox-spec.md new file mode 100644 index 0000000..2f528cd --- /dev/null +++ b/docs/remo-fnox-spec.md @@ -0,0 +1,316 @@ +# Feature Specification: Credential Broker + +**Feature Branch**: `005-credential-broker` +**Created**: 2026-05-23 +**Status**: Draft +**Input**: User description: "Defend against malicious-dependency supply-chain attacks by removing long-lived developer credentials from Remo instances. Replace ambient environment variables and dotfile-resident secrets with an on-instance broker process that surfaces narrowly-scoped, allowlisted credentials into each devcontainer via per-project Unix sockets." + +## Background and Motivation + +Recent supply-chain attacks against npm (Shai-Hulud, Mini Shai-Hulud against `@antv`), PyPI, and other ecosystems share a pattern: a malicious dependency's install or postinstall script runs with the developer's full ambient environment and reads `GITHUB_TOKEN`, `NPM_TOKEN`, `AWS_*`, `~/.aws/credentials`, `~/.netrc`, `~/.npmrc`, SSH private keys, and similar. The Remo instance — being where most active dev credentials accumulate — is a high-value target. + +Remo's existing isolation (one instance per developer, separate from the laptop) bounds blast radius, but the *contents* of the instance are exactly the secrets the attacker wants. Every `npm install`, `cargo build`, `pip install` of an untrusted package today runs with full credential access. + +The credential broker design closes this gap by ensuring: + +1. Long-lived developer credentials never live on the Remo instance at rest. +2. The instance fetches credentials on demand from an external backend (1Password, Vault, AWS Secrets Manager, etc.). +3. Each devcontainer sees only the credentials the project it hosts has explicitly declared a need for, enforced by kernel-level namespace separation, not just policy. + +## Terms and Definitions + +These terms are used consistently throughout this spec and should be adopted in all related code, CLI surface, and documentation. + +| Term | Definition | +|---|---| +| **Backend** (L0) | The external secret store of record — 1Password, HashiCorp Vault, AWS Secrets Manager, Azure Key Vault, GCP Secret Manager, age-encrypted file in a private repo, or an OS keychain. Holds the user's actual credential values. Never directly accessed by devcontainers or untrusted code. | +| **Node** (L1) | A bare-metal or VM-based machine that hosts one or more Remo *instances*. Only applicable to self-hosted providers (Incus, Proxmox). For AWS and Hetzner, the cloud provider owns this layer and Remo does not address it. A node may host multiple instances belonging to one or many users. | +| **Instance** (L2) | The compute resource Remo creates per `remo create`. The SSH target tracked in `known_hosts.yml`. Called *instance* (AWS), *server* (Hetzner), or *container* (Incus, Proxmox) in per-provider CLI labels — but structurally a single concept. The *broker* runs here. | +| **Devcontainer** (L3) | A Docker container launched inside an *instance* by `devcontainer up`, one per *project*, where untrusted dependency code (npm install, etc.) actually executes. Consumes one *project socket*. | +| **Project** (L4) | A directory under `~/projects/` on an instance, usually a git repo, typically containing a `.devcontainer/` config and a *project manifest*. The unit selected in the project menu. | +| **Backend** identity | A credential the *broker* uses to authenticate upward to the backend (e.g., a 1Password Service Account token, Vault AppRole, AWS IAM instance profile). Scoped narrowly to a single *instance*'s allowed secrets. Not the same as the secrets it fetches. | +| **Bootstrap token** | The on-disk form of the backend identity stored on an instance (file at `/etc/remo-broker/bootstrap-token` on the instance). For self-hosted providers, originates on the *node* and is bind-mounted in. For Hetzner, SSH-pushed at create time. For AWS, replaced by an instance profile (no on-disk token). | +| **Broker** | The Remo-owned daemon (`remo-broker`) running on the instance as a systemd unit. Holds the bootstrap token, fetches credentials from the backend (using `fnox` internally as its multi-backend retrieval library), enforces per-project allowlists, and serves *project sockets* to devcontainers. One broker per instance. See "Component Sourcing" below for why this is built rather than adopted. | +| **Project socket** | A Unix domain socket at `/run/remo-broker/.sock` on the instance, one per active project, bind-mounted as `/run/remo-broker/sock` into the project's devcontainer. The broker enforces a per-project allowlist on each. | +| **Project manifest** | `.remo/broker.toml` (auto-synthesized by Remo) or `.devcontainer/remo-broker.toml` (committed to the repo) declaring the set of backend-resolvable secret *names* this project is permitted to fetch. The broker reads this when creating the project socket and uses it as the per-project allowlist. Backend-side mappings (which credential store each name lives in, what backend identity to use) are configured separately at the instance level via the embedded fnox layer's own configuration. | +| **Provisioning credential** | A credential Remo uses to call cloud APIs (HETZNER_API_TOKEN, AWS_ACCESS_KEY_ID, Incus/Proxmox API tokens, 1Password SA admin token). Lives only in the laptop's fnox configuration, fetched on demand per `remo` invocation, never persisted to an instance or node. | +| **User secret** | A credential a project needs at runtime (GITHUB_TOKEN, NPM_TOKEN, runtime AWS keys, ANTHROPIC_API_KEY). Fetched on demand by the broker via the bootstrap token, held in broker memory only, exposed to devcontainers via project sockets. Never written to disk on the instance. | + +### Layer diagram + +``` +L0 Backend + └── 1Password / Vault / AWS Secrets Manager / age+git / keychain + ▲ + │ broker authenticates with bootstrap token + │ +L1 Node (Incus/Proxmox only; absent on AWS/Hetzner) + ├── /var/lib/remo-broker/instance-tokens/ + └── bind-mounted read-only into the instance + │ + ▼ +L2 Instance + ├── /etc/remo-broker/bootstrap-token (file or IMDS-derived) + ├── remo-broker daemon (systemd unit; embeds fnox for backend retrieval) + ├── /run/remo-broker/projA.sock (allowlist: GITHUB_TOKEN, …) + └── /run/remo-broker/projB.sock (allowlist: NPM_TOKEN, …) + │ + ▼ (bind-mounted as /run/remo-broker/sock) +L3 Devcontainer (one per project) + └── tools fetch secrets via the mounted project socket only + │ + ▼ +L4 Project workspace (mounted from ~/projects/) +``` + +For AWS/Hetzner, L1 collapses: there is no Remo-addressable node, and the bootstrap is delivered either via cloud workload identity (AWS instance profile) or SSH push (Hetzner). + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Project creds available in devcontainer, never on the instance OS (Priority: P1) + +A developer SSHes into a Remo instance, picks a project from the menu, and lands in a devcontainer. Inside the devcontainer, `gh auth status` shows them authenticated, `npm publish` finds an NPM_TOKEN, `aws sts get-caller-identity` works. None of those credentials existed on the instance OS before the devcontainer started, and none persist after it stops. A worm in `npm install` running inside that devcontainer can only see the credentials the project's manifest declared a need for — not credentials from other projects, not the bootstrap token, not the developer's full secret backend. + +**Why this priority**: This is the entire point of the feature. Without it, no other behavior matters. + +**Independent Test**: Provision an instance, configure a project manifest declaring `GITHUB_TOKEN`, launch the devcontainer, verify `gh auth status` reports authenticated *and* `cat ~/.config/gh/hosts.yml` shows no on-disk token. From outside the devcontainer (on the instance OS), verify `printenv` shows no GITHUB_TOKEN and `~/.config/gh/` does not exist for the project's user. + +**Acceptance Scenarios**: + +1. **Given** an instance with the broker installed and a project with a manifest declaring `GITHUB_TOKEN`, **When** the user selects that project from the menu, **Then** the launched devcontainer can use `gh` against authenticated GitHub APIs. +2. **Given** the same setup, **When** the user runs `printenv` or inspects `~/.aws/`, `~/.npmrc`, `~/.netrc` on the instance OS (outside any devcontainer), **Then** none of the project's credentials appear. +3. **Given** two projects A and B where A's manifest declares `GITHUB_TOKEN` and B's declares `NPM_TOKEN`, **When** project A's devcontainer requests `NPM_TOKEN` via the broker, **Then** the request is denied. +4. **Given** a devcontainer is running with a project socket mounted, **When** the devcontainer process exits, **Then** the project socket is removed from `/run/remo-broker/` and the broker drops the project's allowlist from memory. + +--- + +### User Story 2 - Multi-device access to the same instance (Priority: P1) + +A developer creates an instance from their laptop, does some work, then later runs `remo shell` from a different machine (a second laptop, a web session, a phone-tethered tablet). The instance is reachable and project credentials work in devcontainers from any device, without re-bootstrapping the instance or re-unlocking anything device-specific. + +**Why this priority**: A broker design that only worked from the laptop that created the instance would fail Remo's core "remote dev environment" promise. The broker lives on the instance precisely to make this work. + +**Independent Test**: Create an instance from device A, run a devcontainer with broker creds successfully. Without reconnecting from device A, run `remo shell` from device B against the same instance, launch the same devcontainer, verify credentials still work. + +**Acceptance Scenarios**: + +1. **Given** an instance was created from device A, **When** the user runs `remo shell` from device B, **Then** the broker is already running and serves credentials to launched devcontainers without intervention. +2. **Given** the instance has been rebooted, **When** the broker comes back up, **Then** it re-reads its bootstrap token, re-authenticates to the backend, and resumes serving without any device needing to connect. +3. **Given** an autonomous Claude Code session was started in a devcontainer before the user disconnected, **When** the developer is offline overnight, **Then** the session continues to access its allowlisted credentials. + +--- + +### User Story 3 - Provisioning credentials never reach the instance (Priority: P1) + +A developer runs `remo hetzner create myproject`. Remo reads the Hetzner API token from the laptop's fnox (not from `HETZNER_API_TOKEN` in the laptop's shell env), uses it to provision the VM, and then forgets it. After provisioning, the Hetzner API token has never been written to the instance, never appeared in cloud-init user-data visible in the Hetzner console, and is not present in the laptop's process environment. + +**Why this priority**: Provisioning credentials are typically the most powerful (can create/destroy any resource in the account) and the easiest to leak. The instance has no need for them. + +**Independent Test**: With `HETZNER_API_TOKEN` *unset* in the laptop shell but stored in the laptop's fnox config, run `remo hetzner create test`, then `ssh remo@test "env | grep -i hetzner"` returns nothing and the Hetzner console's user-data field for the VM contains no token. + +**Acceptance Scenarios**: + +1. **Given** Hetzner API token is stored in the laptop's fnox and *not* exported in the laptop's environment, **When** `remo hetzner create` runs, **Then** provisioning succeeds. +2. **Given** an instance has been created, **When** the user inspects cloud provider metadata/user-data via the provider's console or API, **Then** no provisioning credentials are visible. +3. **Given** the same setup, **When** the user inspects the instance OS, **Then** no provisioning credentials are present in environment, dotfiles, or any disk location. + +--- + +### User Story 4 - Per-project credential allowlist via the manifest (Priority: P2) + +A developer clones a new repo into `~/projects/foo`. Selecting it from the menu either reads the committed `.devcontainer/remo-broker.toml`, or — if absent — synthesizes a default `.remo/broker.toml` declaring only `github_token` (enough for `git push`). The developer can broaden the allowlist by editing the manifest; secrets not in the manifest cannot be fetched, even by tools that know their names. + +**Why this priority**: The allowlist is the policy mechanism that makes the kernel-enforced isolation actually meaningful. Without it, every devcontainer would see every secret. + +**Independent Test**: Place a project with a manifest declaring only `github_token`. Inside the devcontainer, attempt to fetch `npm_token` via the broker socket (`remo-broker get npm_token`) — expect refusal. Add `npm_token` to the manifest, restart the devcontainer, retry — expect success. + +**Acceptance Scenarios**: + +1. **Given** a project with no manifest, **When** the user selects it from the menu, **Then** Remo synthesizes `.remo/broker.toml` with a minimal default allowlist (gitignored) and proceeds. +2. **Given** a project with `.devcontainer/remo-broker.toml` declaring `[mcp] secrets = ["x", "y"]`, **When** the devcontainer requests secret `z`, **Then** the broker returns a denial and logs the attempt. +3. **Given** the manifest is updated, **When** the devcontainer is rebuilt or restarted, **Then** the new allowlist takes effect. + +--- + +### User Story 5 - Bootstrap token rotation and instance destruction revoke access (Priority: P2) + +When `remo destroy` runs against any instance, Remo revokes the bootstrap token at the backend *before* destroying the instance. A long-running rotation policy also periodically replaces each instance's bootstrap token. A token leaked from a destroyed or rotated-away instance has no remaining backend access. + +**Why this priority**: Without revocation, the broker design just relocates the attack surface from "credentials on disk" to "bootstrap tokens that live until manually rotated." Lifecycle integration is what makes the threat model honest. + +**Independent Test**: Create an instance, save a copy of its bootstrap token externally. Destroy the instance. Use the saved token to attempt to fetch a secret — expect failure within the backend's revocation propagation window (typically seconds). + +**Acceptance Scenarios**: + +1. **Given** an instance with an active bootstrap token, **When** `remo destroy` runs, **Then** the token is revoked at the backend before any instance-deletion API call. +2. **Given** a rotation policy is configured, **When** the rotation interval elapses, **Then** a fresh bootstrap token is provisioned to the instance and the previous one is revoked. +3. **Given** rotation fails for any reason, **When** the broker detects auth failures against the backend, **Then** it surfaces an actionable error and continues serving in-memory-cached credentials until they expire. + +--- + +### User Story 6 - Devcontainer auto-synthesis for projects without one (Priority: P2) + +The current project menu launches `devcontainer up` only when a project contains `.devcontainer/devcontainer.json`; otherwise it falls back to a plain shell on the instance OS, defeating the credential boundary. With this feature, projects without a committed devcontainer get an auto-synthesized `.remo/devcontainer.json` based on simple language detection, so *every* project the user selects from the menu lands them in a devcontainer with a broker socket. + +**Why this priority**: The instance-OS fallback is the leak that defeats the rest of the design. Required for correctness. + +**Independent Test**: Clone a repo with no `.devcontainer/` and a `package.json`. Select it from the menu. Expect to land in a Node-based devcontainer with a project socket mounted, not in a shell on the instance OS. + +**Acceptance Scenarios**: + +1. **Given** a project with no devcontainer config and a recognized language marker (`package.json`, `Cargo.toml`, `pyproject.toml`, etc.), **When** the user selects it, **Then** Remo writes `.remo/devcontainer.json` matching the language and launches it. +2. **Given** a project with no language marker, **When** the user selects it, **Then** Remo uses a generic base image and proceeds. +3. **Given** the user explicitly wants a shell on the instance OS, **When** they pick the "exit to host shell" menu option, **Then** they get one — with a one-time warning that no broker is available there. + +--- + +### Edge Cases + +- **Backend unavailable**: broker holds in-memory-cached secrets until they expire, then surfaces clear errors to devcontainers. Does not silently return stale values past expiry. +- **Backend unreachable from instance (network issue)**: broker reports the error via the project socket; tools using the broker see a clear "credential unavailable" error rather than a generic auth failure. +- **Two projects with the same name in different parent dirs**: project socket naming uses the absolute project path's hash (truncated) as suffix to avoid collisions. +- **A devcontainer escape into the instance OS**: attacker gains access to every project socket currently mounted in the instance plus the bootstrap token. Rotation interval and per-instance scoping bound the damage; full-backend access is not possible. +- **A node-level compromise on Incus/Proxmox**: attacker gains every instance's bootstrap token on that node. Threat model treats nodes as critical assets requiring OS hardening separate from instances. +- **A user runs untrusted code on the instance OS rather than in a devcontainer** (via "exit to host shell"): no broker available, so user secrets are simply not present. The escape hatch is safe by absence. +- **A project manifest declares a secret that doesn't exist in the backend**: broker returns a "not found" error to the devcontainer; tool's behavior depends on the tool, but the broker itself does not crash or fall back to other secrets. +- **The user has chosen `age + git` as backend** (no per-instance scoping primitive): `remo init` warns that this backend does not support narrowly-scoped bootstrap tokens; either reject the combination or fall back to laptop-unlock-per-session for Hetzner/Incus/Proxmox. +- **AWS SSM access mode**: bootstrap (instance profile) is metadata-based, no SSH push needed; broker installation still proceeds via the standard `*_configure.yml` flow which already works over SSM. + +## Requirements *(mandatory)* + +### Functional Requirements + +**Backend integration** +- **FR-001**: System MUST support pluggable backends for the credential store, with first-class support for 1Password, HashiCorp Vault, AWS Secrets Manager, and age-encrypted git for the v1 cut. +- **FR-002**: System MUST allow per-installation backend choice via `remo init`, persisted to laptop-side fnox configuration. +- **FR-003**: System MUST warn the user at `init` time when their selected backend lacks per-instance scoping primitives (age + git) and offer a more secure alternative or a clearly-described downgrade. + +**Provisioning credentials** +- **FR-004**: System MUST read provisioning credentials (HETZNER_API_TOKEN, AWS access keys, Incus/Proxmox API tokens) from the laptop's fnox rather than the laptop's shell environment. +- **FR-005**: System MUST NOT write provisioning credentials to any instance's disk, environment, or cloud-init user-data. +- **FR-006**: System MUST replace `lookup('env', '')` patterns in `ansible/group_vars/all.yml` with `lookup('pipe', 'fnox get ...')` invocations against the laptop's fnox. + +**Bootstrap & broker installation** +- **FR-007**: System MUST install the broker (`remo-broker` daemon + systemd unit; Remo-owned, embeds fnox for backend retrieval) on every Remo instance as part of the standard `*_configure.yml` Ansible flow. +- **FR-008**: For AWS, system MUST attach an instance-scoped IAM role at create time and configure the broker to use IMDS for credentials. No bootstrap token shall be written to disk. +- **FR-009**: For Hetzner, system MUST mint an instance-scoped bootstrap token on the laptop, SSH-push it to `/etc/remo-broker/bootstrap-token` (mode 0400, root) after first boot, and not include the token in any cloud-init user-data. +- **FR-010**: For Incus and Proxmox, system MUST mint the bootstrap token on the laptop, place it under `/var/lib/remo-broker/instance-tokens/` on the *node* (not in any instance), and bind-mount it read-only into the instance at `/etc/remo-broker/bootstrap-token`. The node itself MUST NOT inspect or store project information. +- **FR-011**: System MUST register the node (one-time per Incus/Proxmox node) via a new `remo incus add-node` and `remo proxmox add-node` command, installing the token-manager helper. + +**Project sockets and manifests** +- **FR-012**: System MUST discover project manifests in priority order: `.devcontainer/remo-broker.toml` (committed) → `.remo/broker.toml` (auto-synthesized, gitignored). +- **FR-013**: System MUST auto-synthesize a minimal default manifest declaring `github_token` for projects with no existing manifest. +- **FR-014**: System MUST create one project socket per active project at `/run/remo-broker/.sock` on the instance, enforcing the project's manifest allowlist. +- **FR-015**: System MUST mount the appropriate project socket into the project's devcontainer at `/run/remo-broker/sock` via devcontainer bind-mount configuration. +- **FR-016**: System MUST remove a project socket when its associated devcontainer exits. + +**Devcontainer enforcement** +- **FR-017**: The project menu MUST launch every selected project inside a devcontainer (committed or auto-synthesized), with no fallback to running the project on the instance OS. +- **FR-018**: The project menu MUST provide an explicit "exit to instance shell" option that surfaces a one-time warning explaining the broker is not available outside a devcontainer. +- **FR-019**: The instance OS shell MUST NOT have a broker socket available by default. + +**Lifecycle** +- **FR-020**: `remo destroy` MUST revoke an instance's bootstrap token at the backend *before* destroying the instance. +- **FR-021**: System MUST support periodic rotation of bootstrap tokens via a `remo rotate-bootstrap [instance]` command, configurable to run automatically on a cadence. +- **FR-022**: The broker MUST hold user-secret values in memory only, never writing them to disk on the instance. + +**Auditability** +- **FR-023**: The broker MUST log every secret-access request (project, secret name, allowed/denied, timestamp) to a local log file readable only by root on the instance. +- **FR-024**: System MUST provide a `remo audit ` command that retrieves and displays the broker's access log. + +### Non-Functional Requirements + +- **NFR-001**: A broker-mediated secret fetch in the steady state (warm cache) MUST add no more than 50 ms latency over a direct env-var read. +- **NFR-002**: The broker MUST survive `systemd` restarts and instance reboots without manual reconfiguration. +- **NFR-003**: An instance's broker MUST function for all configured backends across a backend network outage by serving the last in-memory-cached value until that value's TTL expires. + +### Key Entities + +- **Node** (new model): represents an Incus or Proxmox node registered with Remo. Fields include `name`, `host` (SSH target), `provider`, `bootstrap_admin_identity` (an SA admin token capable of minting per-instance sub-tokens, stored in laptop's fnox). Not used for AWS or Hetzner. +- **Bootstrap token** (no Remo model — opaque string): the per-instance backend identity. Located at `/etc/remo-broker/bootstrap-token` on instance, `/var/lib/remo-broker/instance-tokens/` on node (self-hosted only). On instances with a TPM, the token SHOULD be sealed at rest via systemd's `LoadCredentialEncrypted` / TPM2 binding so that an offline disk read does not yield a usable token. +- **Project manifest** (TOML file in repo or `.remo/`): declares the set of backend secret names this project requires. Read by the broker when minting a project socket. +- **Project socket** (Unix domain socket): per-project, per-instance, ephemeral, enforces the manifest's allowlist. +- **Broker** (process): one per instance, runs as a systemd unit (`remo-broker.service`), Remo-owned code that holds the bootstrap token, embeds fnox as its multi-backend retrieval library, holds a memory-only TTL cache of recently-resolved user secrets, enforces per-project allowlists, and writes an append-only audit log. +- **KnownHost** (existing model, unchanged): continues to represent L2 instances and their SSH-target metadata. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: After feature is fully adopted, **zero long-lived user secrets** are written to any Remo instance's disk. Verified by auditing `~/.aws/`, `~/.config/gh/`, `~/.npmrc`, `~/.netrc`, and `env` across a representative sample of instances. +- **SC-002**: A simulated supply-chain attack — a malicious `postinstall` script running `printenv`, `cat ~/.aws/credentials`, `cat ~/.config/gh/hosts.yml`, and an outbound HTTP exfil call from inside a devcontainer — recovers **only** the secrets declared in that project's manifest, and no others. +- **SC-003**: `remo shell` works from any device the user has authenticated to the backend from, with **no per-device instance reconfiguration** required. +- **SC-004**: Provisioning a new instance and launching a devcontainer adds **no more than 30 seconds** to today's flow on a typical broadband connection (warm laptop fnox cache, warm backend). +- **SC-005**: After `remo destroy`, a copy of the destroyed instance's bootstrap token is **rejected by the backend within 60 seconds**. +- **SC-006**: When the user adds a secret to a project manifest, it becomes available inside that project's devcontainer **on the next devcontainer restart**, with no instance-level configuration changes. + +## Component Sourcing + +This section records why the broker is built rather than adopted, and which external components are leveraged. + +### Adopted: fnox (laptop CLI + on-instance retrieval library) + +[`fnox`](https://github.com/jdx/fnox) is a mature (v1.25.1, ~39 releases, post-1.0) multi-backend secret-fetching CLI that already abstracts 1Password, HashiCorp Vault, AWS Secrets Manager, age, and the OS keychain behind a single `fnox get ` interface. Remo adopts fnox in two roles: + +1. **Laptop-side provisioning credential lookup** — `lookup('pipe', 'fnox get …')` invocations in Ansible replace `lookup('env', …)` patterns (FR-006). The laptop already typically has fnox configured for the developer's day-to-day secret access. +2. **On-instance backend retrieval inside the broker** — the `remo-broker` daemon embeds fnox (as a subprocess or library, design-deferred) to resolve a name like `GITHUB_TOKEN` to a fetched value via whichever backend is configured for that instance. + +This sidesteps the largest single body of work in the spec — writing and maintaining adapters for five different secret stores, each with its own auth model and SDK. + +### Built: the `remo-broker` daemon + +A broader survey of credential-broker-adjacent tooling (Vault Agent, OpenBao Agent, SPIFFE/SPIRE, Teleport Machine ID, Infisical Agent, systemd credentials, 1Password Connect, EKS Pod Identity Agent, Doppler, sops, Bitwarden Secrets Manager, aws-vault) found **no single existing tool that combines** the four properties this spec requires: + +1. Multi-backend retrieval (1Password + Vault + AWS SM + age + keychain in one process). +2. Per-project Unix-socket serving (one socket per project, distinct allowlists). +3. Manifest-driven allowlists that the broker itself enforces. +4. Bootstrap modes spanning IMDS (AWS), token-file (Hetzner), and node-bind-mount (Incus/Proxmox). + +The closest single-tool fit is **Vault Agent / OpenBao Agent**, but it is single-backend (Vault-only) and its per-listener `role` / `require_request_header` knobs are anti-SSRF controls rather than per-project secret allowlists — true allowlisting would require one agent process per project, which defeats the operational model. + +The strongest patterns to *borrow* (not adopt wholesale) come from: + +- **EKS Pod Identity Agent** — proves the "token-file mounted into client → daemon returns only that client's allowed credentials" pattern in production. +- **SPIFFE/SPIRE Workload API** — proves that kernel-attested per-caller scoping over a Unix socket is practical. + +The broker therefore is Remo-owned code, with these influences guiding its IPC and attestation design. It is small (estimated low single thousands of lines), policy-driven, and depends on fnox-core for the genuinely complex backend-integration work. + +#### Repository layout and implementation language + +The `remo-broker` daemon lives in a separate repository (`get2knowio/remo-broker`) for reasons of language asymmetry (Rust vs. Remo's Python), distribution shape (signed binary releases vs. PyPI), release cadence (the daemon is install-once, the laptop CLI iterates), and audit surface. Implementation language: **Rust**, with [`fnox-core`](https://crates.io/crates/fnox-core) (MIT, published by the fnox author) as a Cargo dependency — closes OQ-7 in the "library" direction by avoiding subprocess fork/exec per uncached fetch and keeping secret values inside fnox-core's typed wrappers end-to-end. + +The new repo owns the broker's internal design and the two cross-repo contracts: + +- `specs/001-broker-daemon/spec.md` — full feature spec for the daemon +- `docs/manifest-schema.md` — versioned TOML schema for `remo-broker.toml` (cross-repo contract; source of truth here, JSON Schema published per release and consumed by Remo for laptop-side validation) +- `docs/wire-protocol.md` — project-socket and admin-socket wire protocol (cross-repo contract) + +Schema-drift mitigations between the two repos: (1) JSON Schema generated from Rust types and validated on the Remo side, (2) `schema_version` integer in every manifest with broker-side refusal of unknown versions, (3) end-to-end CI test exercising both repos against a real manifest + socket round-trip. + +### Future escape hatches + +- If fnox becomes unmaintained or unsuitable, the natural swap is **OpenBao Agent** (MPL-2.0) running one agent per project behind a thin Remo supervisor. Trade-off: loss of native 1Password / OS-keychain support unless those are proxied via OpenBao secret engines. +- If a generic broker daemon emerges upstream that meets all four requirements above, Remo's broker can be retired in favor of it. + +### Complementary, used underneath the broker + +- **systemd `LoadCredentialEncrypted` + TPM2** — used to seal the bootstrap-token file at rest on TPM-equipped instances (typically Incus / Proxmox nodes), so an offline disk read does not yield a usable token. Does not replace any broker function — it hardens token storage. + +## Out of Scope + +- **Sandboxing untrusted code beyond the devcontainer boundary**: this spec does not require additional in-devcontainer sandboxing (gVisor, firejail, network egress restrictions per-install-script). Those are complementary defenses worth considering separately. +- **Replacing existing SSH key management**: the broker handles user secrets fetched at runtime by devcontainer tooling. SSH key material for `remo shell` itself remains handled by the existing flow (laptop ssh-agent forwarding or instance-resident keys, depending on access mode). +- **Backend selection UI improvements** beyond the minimum needed at `remo init`. A full backend management TUI is future work. +- **Secret rotation at the user-secret level**: this spec rotates *bootstrap tokens* (the broker's identity). Rotation of the actual GitHub PAT, NPM token, etc. is the backend's concern and the user's policy choice. +- **Multi-user instances**: this spec assumes each instance has one developer user. Multi-tenant instances would need per-user project-socket isolation, deferred to future work. + +## Open Questions + +- **OQ-1**: Should the project socket be created per-devcontainer-lifetime or per-project-lifetime? The former gives strict ephemerality but may complicate background tasks like `cargo build` running across `devcontainer exec` invocations. The latter is operationally simpler. +- **OQ-2**: For Incus/Proxmox nodes that host instances from multiple developers, how is the node's admin identity (used to mint sub-tokens for each developer's instances) bootstrapped? Per-developer admin SAs, or a single node admin SA with logical sub-scoping? +- **OQ-3**: Should the broker also serve the `gh` git credential helper protocol natively, or is `gh auth login --with-token` against a broker-fetched token sufficient? +- **OQ-4**: Default rotation cadence for bootstrap tokens — 24h, 7d, or "never (revoke only on destroy)"? Trade-off is operational noise vs. exposure window. +- **OQ-5**: How should the broker behave when a backend requires interactive auth (e.g., 1Password biometric prompt) and the requesting context is non-interactive (autonomous AI agent at 3am)? Refuse and surface the requirement, or rely entirely on non-interactive backend identities (SA tokens) and forbid interactive ones for instance use? +- **OQ-6**: Should the broker make the bootstrap-token file's TPM2 sealing mandatory on instances where a TPM is available, or remain opt-in via configuration? Mandatory closes the offline-disk-read attack reliably; opt-in avoids surprises for users on older hardware or with custom systemd setups. +- **OQ-7**: Should the broker embed fnox as a Rust library (tight coupling, one process, faster) or shell out to the `fnox` binary as a subprocess (loose coupling, version-independent, slower)? Subprocess is simpler to ship and lets users upgrade fnox independently; library is faster and avoids a fork+exec per uncached fetch but locks the broker to a specific fnox version. +- **OQ-8**: What's the wire protocol on the project socket — a simple line-based `GET \n` / `\n` shape, a fnox-CLI-compatible protocol (so tools that already speak fnox work unchanged), or something richer (gRPC, JSON-RPC) that supports streaming, watches, and structured errors? Simpler is easier to audit; richer enables future features like credential-change notifications. diff --git a/notifier/Dockerfile b/notifier/Dockerfile new file mode 100644 index 0000000..44e6e8b --- /dev/null +++ b/notifier/Dockerfile @@ -0,0 +1,38 @@ +# Remo Notifier — multi-stage build. +# +# All COPY paths are relative to the build-context root. The context root is +# either the repo root (local builds: `docker build -f notifier/Dockerfile .`) +# or the on-host copy of the bundled context (remo_notifier Ansible role), which +# reproduces the same layout. See specs/007-notifier-sidecar (finding U1). + +# ---- Stage 1: builder ------------------------------------------------------ +FROM python:3.13-slim AS builder +WORKDIR /build +RUN pip install --no-cache-dir uv +# pyproject.toml + README.md + source are required to install the project. +# uv.lock is included for an optional `uv sync --frozen` locked build; a plain +# `uv pip install` does not consume it (finding I1). +COPY pyproject.toml uv.lock README.md ./ +COPY src/ ./src/ +# The remo-cli wheel force-includes ansible/ (and the Dockerfile lives under +# notifier/), so both must be present in the build context for hatchling to +# resolve its forced includes during install. +COPY ansible/ ./ansible/ +COPY notifier/ ./notifier/ +RUN uv venv /opt/venv && \ + . /opt/venv/bin/activate && \ + uv pip install --no-cache-dir ".[notifier]" + +# ---- Stage 2: runtime ------------------------------------------------------ +FROM python:3.13-slim +RUN groupadd --system --gid 65532 notifier && \ + useradd --system --uid 65532 --gid notifier --no-create-home \ + --shell /usr/sbin/nologin notifier +COPY --from=builder /opt/venv /opt/venv +ENV PATH="/opt/venv/bin:$PATH" +USER 65532:65532 +EXPOSE 18181 +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD python -c "import urllib.request; urllib.request.urlopen('http://127.0.0.1:18181/v1/health', timeout=2)" +ENTRYPOINT ["remo-notifier"] +CMD ["serve", "--config", "/etc/notifier/notifier.toml"] diff --git a/pyproject.toml b/pyproject.toml index 7933c52..77c080b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,13 +21,25 @@ dependencies = [ dev = [ "pytest", "pytest-mock", + "pytest-asyncio", + "pytest-cov", + "httpx", "ruff", "mypy", ] +notifier = [ + "fastapi>=0.115", + "uvicorn[standard]>=0.32", + "pydantic>=2.9", + "python-telegram-bot>=21.6", + "structlog>=24.4", + "tomli>=2.0; python_version<'3.11'", +] [project.scripts] remo-cli = "remo_cli.cli.main:cli" remo = "remo_cli.cli.main:cli" +remo-notifier = "remo_cli.notifier.cli:main" [tool.hatch.build.targets.wheel] packages = ["src/remo_cli"] @@ -37,7 +49,14 @@ packages = ["src/remo_cli"] [tool.pytest.ini_options] testpaths = ["tests"] +asyncio_mode = "auto" [tool.ruff] src = ["src"] line-length = 100 + +[tool.uv] +# Supply-chain cooldown: ignore distributions uploaded in the last 7 days so a +# malicious release has time to be detected/yanked before it can be resolved. +# Relative (rolling) window; relies on PyPI's PEP 700 upload-time metadata. +exclude-newer = "7 days" diff --git a/specs/007-notifier-sidecar/addendum-001-standing-grants.md b/specs/007-notifier-sidecar/addendum-001-standing-grants.md new file mode 100644 index 0000000..17f6462 --- /dev/null +++ b/specs/007-notifier-sidecar/addendum-001-standing-grants.md @@ -0,0 +1,338 @@ +# Addendum 001 — Standing Grants ("Always" auto-approval) + +**Parent spec**: [`spec.md`](./spec.md) (Notifier Sidecar, spec 007) +**Status**: Draft (design only — no implementation) +**Created**: 2026-05-31 +**Depends on**: the implemented v1 notifier (FR-001…FR-034, FR-003a, FR-010a). + +## Purpose + +Add a third decision to the approval flow: alongside **Approve** / **Deny**, the +human can choose **Always** — auto-approving the *class* of operation this +request belongs to, so future matching requests are answered without a Telegram +round-trip. + +This extends the per-event model (today every tap approves exactly one +`approval_id`) with a **standing grant**: a human-authored, deterministic rule +the notifier matches on intake. + +## Architecture decision (why the grant lives in the notifier, for now) + +agentsh's `api` approval mode **delegates the approve decision to the notifier** +(it is the registered REST approver). An approver answering from a human's prior +standing instruction is within that delegated authority — it is not a shadow +policy, it is the approver having memory. agentsh has no path today for an +approver to hand a rule back into its signed/versioned policy, so the only +buildable home for the grant is the notifier. + +Consequences this addendum accepts deliberately: + +- The notifier becomes **`allow`-capable**. Today a notifier fault fails secure + (no prompt → deny); with grants, a matcher fault can manufacture `allow`. The + matcher is therefore the most security-critical code in the service and MUST + be deterministic and exact (see FR-G4). +- Standing grants live **outside** agentsh's audited policy, so the notifier's + own list / revoke / audit surface is the backstop (FR-G7, FR-G8). +- The v1 grant store is **in-memory, scoped, and TTL-bounded** — it fails closed + on restart (you get re-asked), avoiding an on-disk security datastore. This + keeps the "no durable state" property of the parent spec intact. + +### Forward compatibility (promotion to agentsh policy) + +The grant predicate is **shaped like an agentsh policy rule** (its +`paths`/`operations`/`match`/host+port grammar). If agentsh later grows an +"approver returns a signed rule" capability, a notifier grant promotes straight +into agentsh's signed, versioned, audited policy with no reshaping — moving the +grant from "the approver remembers" up to "it's in the audited policy," the +better long-term home. This addendum does not require that agentsh change; it +only keeps the door open. + +## Decisions captured (from design discussion) + +- **D1**: Grant enforcement lives in the notifier (in-memory) for v1, under the + delegated-approver model above. Not agentsh-side yet (no such capability). +- **D2**: **Everything is generalizable** in v1 — no category that is ineligible + for "Always." The schema carries an `eligible` flag so a denylist + (destructive / egress / secrets) can be added later via config with no + protocol change. +- **D3**: Grants are **in-memory, scoped, TTL-bounded, fail-closed**. No + cross-restart persistence in v1. +- **D4**: Grant predicates are agentsh-rule-shaped for future promotion (D-fwd). +- **D5**: The matcher is deterministic/exact; the *intelligence* (proposing the + class) happens once, at tap-time, with the human confirming. + +## Clarifications + +### Session 2026-06-01 + +- Q: Does a single "Always" cover a bundle of related syscall-level events, and how is the bundle identified? → A: No bundling in v1. The matcher is strictly per-operation; breadth comes from the human choosing a broader predicate, with `policy_rule_name` available as the broadest candidate rung (folds into FR-G6, not a new primitive). +- Q: Should "Always" be offered on first encounter or only after the class repeats N times? → A: Offer on every prompt; rely on tightest-first candidates (FR-G6) and narrow default scope (FR-G7) as guardrails. Repeat-gated offering is a future enhancement. +- Q: Single global default TTL or per-grant lifetime choice? → A: Single configurable `default_ttl_seconds` (default 8h) applied to every grant; no "until revoked" / indefinite grants in v1. Per-grant lifetime choice deferred. +- Q: How are active grants listed — bridge read endpoint, or Telegram/SSH only? → A: No bridge read endpoint in v1 (avoids disclosing the auto-approve set to co-located containers). Telegram `/rules` is the only list+revoke surface; `GET /v1/grants` and the `remo notifier rules` CLI are dropped/deferred (loopback-only admin port is the future option). + +## User Scenarios + +### US-G1 — Human grants "Always" for a class (Priority: P1) + +A request arrives the human has seen before. Instead of Approve, they tap +**Always…**, pick a generalization + scope from a short list, and confirm. This +request is approved *and* a standing grant is created. The next matching request +returns `allow` instantly with no Telegram message. + +**Independent test**: Tap Always on a request; send a second matching request → +it returns `allow` in well under a second with no notification, and the audit +log records the grant id. + +### US-G2 — Human reviews and revokes standing grants (Priority: P1) + +The human runs `/rules` in Telegram and sees active grants (class, scope, age, +TTL, use count). They `/revoke ` (or tap a revoke button); subsequent +matching requests prompt again. + +**Independent test**: Create a grant, `/revoke` it, send a matching request → +the human is prompted again. + +### US-G3 — Auto-approvals stay visible (Priority: P2) + +Auto-approved operations are logged with their grant id, and a periodic digest +("auto-approved N ops via M rules") is delivered so standing grants are never +silent. + +## Functional Requirements (additive) + +### Intake short-circuit + +- **FR-G1**: On `POST /v1/approve`, before reserving a slot or notifying, the + notifier MUST evaluate the request against active standing grants. On a match, + it MUST return `200` with `decision: allow` immediately, without sending any + notification and without occupying a pending slot. +- **FR-G2**: An auto-approval response MUST identify its source: `responder` = + `rule:{grant_id}` and `reason` = a fixed marker (e.g. `auto-approved via + standing grant`). The `grant_id` MUST appear in the response so callers and + audit can correlate. +- **FR-G3**: On no match, the existing flow (reserve → notify → await, FR-001… + FR-010a) applies unchanged. + +### The matcher (allow-capable — highest criticality) + +- **FR-G4**: A request matches a grant **iff** all hold, evaluated + deterministically (no fuzzy/semantic matching at runtime): (a) the grant is + active (not expired, not revoked); (b) the request scope satisfies the grant + scope (FR-G9); (c) the operation satisfies the grant predicate exactly + (FR-G5). Any ambiguity or evaluation error MUST be treated as **no match** + (fail-closed → prompt the human). +- **FR-G5**: The grant predicate MUST be a structured, inspectable rule over the + operation fields (kind; for `command`: command + an args matcher of exact / + prefix / glob; for `file`: path globs + operations; for `network`: host + exact-or-suffix + port; for `signal`: signal + target). It MUST NOT contain + free-form/learned matching. Predicate grammar mirrors agentsh policy rules + (D4). + +### Deriving the class (proposal, at tap-time) + +- **FR-G6**: When the human taps **Always…**, the notifier MUST present a small + ordered set of candidate generalizations — **at most 4** (tightest first) — derived from the + request via deterministic templates per operation kind, each paired with a + scope choice. The broadest candidate rung MAY be a `policy_rule_name`-based + predicate (auto-approve operations agentsh attributes to the same rule, within + scope). The human selects exactly one. The notifier MUST NOT create a grant + broader than what the human selected, and there is **no bundling primitive** — + every grant is matched per-operation (FR-G4/G5). + +### Lifecycle, scope, limits + +- **FR-G7**: Grants MUST carry a **scope** (`session` | `workspace` | `project` + | `instance` | `global`) and the notifier MUST default new grants to the + **narrowest scope that covers the request** unless the human explicitly widens + it. (`global` is permitted in v1 per D2 but is never the default.) +- **FR-G8**: Grants MUST carry a **TTL** and MUST stop matching once expired. + v1 applies a single configurable `default_ttl_seconds` (default 8h) to every + grant; there is **no indefinite / "until revoked" grant** (per-grant lifetime + choice is deferred). Revocation (FR-G10) is always available before expiry. +- **FR-G9**: Grant state MUST be **in-memory only**; a restart loses all grants + and subsequent requests fail closed (re-prompt). The notifier MUST enforce a + configurable **maximum number of active grants**, rejecting new grants beyond + it with a clear Telegram message (mirrors the pending-approval cap, FR-034). + +### Visibility, revocation, audit + +- **FR-G10**: The human MUST be able to **list** active grants and **revoke** any + grant, from Telegram (`/rules`, `/revoke `), with effect on subsequent + requests being immediate. A **global pause** ("disable all auto-approval") MUST + be available. +- **FR-G11**: Every auto-approval MUST be logged with its `grant_id` and the + matched operation's structural metadata (never secrets/bodies — FR-017), and + the notifier SHOULD deliver a periodic digest of auto-approval activity. + +### Configuration + +- **FR-G12**: Configuration MUST include: grants enabled/disabled, + `default_ttl_seconds`, `max_grants`, `allow_global_scope`, and + `digest_interval_seconds` (0 disables the digest). The grant schema MUST carry + an `eligible` flag per class so a future ineligibility denylist (D2) can be + applied without a protocol change. + +## Wire-protocol changes (additive, backward-compatible) + +### `ApprovalResponse` — new optional fields + +- `responder` already exists; auto-approvals set it to `rule:{grant_id}`. +- Add optional `grant_id` (string) — present on auto-approved (FR-G2) and on the + response that *created* a grant (the "Always" tap), echoing the new grant's id. + +### `Grant` (new object) + +Returned to the caller on the response that creates a grant, and the internal +record the matcher evaluates. Agentsh-rule-shaped (D4): + +```json +{ + "grant_id": "uuid", + "created_at": "RFC3339", + "created_by": "telegram:paulofallon", + "expires_at": "RFC3339", + "source_approval_id": "uuid", + "scope": { "type": "project", "value": "myproj" }, + "eligible": true, + "predicate": { + "kind": "command | file | network | signal", + "command": "git", + "args_match": { "type": "prefix", "value": ["push"] }, + "paths": ["{{workspace}}/**"], + "operations": ["delete"], + "host": { "type": "suffix", "value": ".github.com" }, + "port": 443 + }, + "uses_count": 0, + "last_used_at": null +} +``` + +(Only the predicate fields relevant to `kind` are present.) + +### Endpoints + +- `POST /v1/approve` — gains the FR-G1 short-circuit; response may carry + `grant_id`. No breaking change for callers that ignore it. +- **No** new externally-mutating endpoint. Grant creation happens only via a + human Telegram tap; grant revocation happens via Telegram (`/revoke`). This + keeps mutation authenticated to the authorized chat and off the unauthenticated + bridge port. +- **No `GET /v1/grants` in v1** (resolved OQ4): a bridge-bound read endpoint + would disclose the auto-approve set to co-located containers — the very agents + being gated. Listing is **Telegram `/rules` only**. A future host-loopback-only + admin port could back a CLI lister without bridge exposure. + +## Telegram UX + +Inline keyboard becomes a two-tap flow: + +``` +[✅ Approve] [⏩ Always…] [❌ Deny] +``` + +- **Always…** edits the message into a granularity + scope picker built from + FR-G6 candidates, e.g. for `git push origin main` in project `myproj`: + + ``` + Always allow: + [git push * · this project] + [git * · this project] + [git push * · everywhere] + [Cancel] + ``` + + with the default TTL shown (e.g. "expires in 8h"). v1 keeps a single + configurable TTL; per-grant TTL choice is an open question (OQ2). +- Selecting a candidate approves *this* request (allow) **and** creates the grant + (FR-G7/G8). The message is edited to confirm (`⏩ Always: git push * · myproj · + 8h · by @paulofallon`). +- `/rules` lists active grants with inline `[Revoke]` buttons; `/revoke ` + and a `/pause` global toggle (FR-G10). + +## CLI additions + +- **None in v1** (resolved OQ4). Grant listing and revocation are Telegram-only + (`/rules`, `/revoke`, `/pause`). A `remo notifier rules ` command is + deferred until a host-loopback-only admin surface exists, to avoid exposing the + grant set on the bridge. + +## Config additions (sketch) + +```toml +[grants] +enabled = true +default_ttl_seconds = 28800 # 8h +max_grants = 100 +allow_global_scope = true # D2: everything generalizable in v1 +``` + +## Security considerations + +- **Allow-capable matcher** is the crown-jewel risk. Keep it exact and + deterministic; treat any uncertainty as no-match (FR-G4). Test the matcher as + adversarially as the fail-secure paths. +- **Scope defaults narrow** (FR-G7) and **TTL bounds every grant** (FR-G8): the + two compensating controls that carry the safety budget now that everything is + eligible (D2). `global` is opt-in, never default. +- **Fail-closed everywhere**: grant store unavailable, ambiguous match, expired, + or post-restart → prompt, never auto-allow. +- **Revocation + visibility** (FR-G10) is the human's backstop since grants sit + outside agentsh's audited policy. +- **No secrets in grant logs** (FR-017 still applies). +- **Authority**: in v1's single-chat model, the one authorized chat mints and + revokes grants. Multi-chat authority is out of scope (parent spec constraint). + +## Out of scope (this addendum) + +- Cross-restart / on-disk grant persistence (revisit only if "forgot my Always + after redeploy" proves painful). +- Promotion of grants into agentsh's signed policy (future; requires an agentsh + capability that does not exist today — see Forward compatibility). +- A category **ineligibility** denylist (schema-ready via `eligible`, but no + enforced denylist in v1 per D2). +- LLM/semantic class proposal. Candidate generation is deterministic templates + in v1; an LLM could later *suggest* labels but MUST NOT enter the runtime + matcher. +- "Offer Always only after N repeats" behavioral heuristic (nice-to-have; see + OQ3). + +## Open questions + +- **OQ1 (agentsh)**: Does agentsh's `api` approval response have any extension + point, and what is its signing model for programmatically-added rules? This + determines if/when grants can be promoted into signed policy. +- **OQ2 (TTL)**: ✅ Resolved (2026-06-01) — single configurable + `default_ttl_seconds` (default 8h) for every grant; no indefinite grants in + v1. See Clarifications. +- **OQ3 (offer timing)**: ✅ Resolved (2026-06-01) — offer on every prompt; + guardrails are tightest-first candidates + narrow default scope. Repeat-gated + offering deferred. See Clarifications. +- **OQ4 (read exposure)**: ✅ Resolved (2026-06-01) — no bridge read endpoint; + listing is Telegram `/rules` only; CLI lister deferred to a future + loopback-only admin port. See Clarifications. +- **OQ5 (match granularity)**: ✅ Resolved (2026-06-01) — no bundling in v1; the + matcher is per-operation, with `policy_rule_name` available as the broadest + candidate predicate rung. See Clarifications. + +## Success criteria + +- [x] **SC-G1**: After a human grants Always for a class, a subsequent matching + request returns `allow` with no Telegram traffic. *(Verified: short-circuit + returns allow with no transport send — `test_grant_short_circuit_allows`. The + <500 ms bound is an in-memory dict+predicate match, not separately asserted — + accepted finding G2.)* +- [x] **SC-G2**: No request is ever auto-approved except by an active, unexpired, + unrevoked, human-created grant whose deterministic predicate + scope it + satisfies; every other path still prompts or fails closed. *(Verified: + `test_grants.py` matcher suite + server fall-through/disabled/paused tests.)* +- [x] **SC-G3**: A human can list and revoke any standing grant and globally pause + auto-approval, with immediate effect. *(Verified: `test_cmd_rules_revoke_pause`.)* +- [x] **SC-G4**: Every auto-approval is individually auditable by `grant_id`, with + no secrets in the record. *(Verified: `test_auto_approval_audit_log_has_no_secrets`.)* +- [x] **SC-G5**: A restart clears all grants and the system fails closed + (re-prompts). *(Verified: `test_fresh_store_is_empty_restart_fail_closed`.)* + +All SC-G verified via the local test suite (108 notifier tests, 91% coverage). +No live-host/bot acceptance was required for grants — the loop is fully +exercised against a fake transport and a mocked Bot. diff --git a/specs/007-notifier-sidecar/checklists/requirements.md b/specs/007-notifier-sidecar/checklists/requirements.md new file mode 100644 index 0000000..5283ee5 --- /dev/null +++ b/specs/007-notifier-sidecar/checklists/requirements.md @@ -0,0 +1,37 @@ +# Specification Quality Checklist: Notifier Sidecar — Telegram approval bridge for agentsh + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-05-31 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- The source prompt was implementation-heavy (named FastAPI, structlog, Dockerfile contents, file layout, etc.). Those details were intentionally deferred to the planning phase; the spec captures only observable behavior, the durable wire-protocol contract, the fail-secure guarantees, and the operator workflow. +- "Telegram" and "HTTP" appear in the spec as they are intrinsic to the feature's purpose (the human-facing channel and the integration surface), not incidental technology choices. They are treated as product-level constraints, not implementation leakage. +- Three clarifications (restart behavior, single authorized chat, network exposure) were resolved up front from the self-contained source prompt and recorded in the Clarifications section rather than left as markers. +- Items marked incomplete require spec updates before `/speckit.clarify` or `/speckit.plan`. diff --git a/specs/007-notifier-sidecar/contracts/grant-schema.md b/specs/007-notifier-sidecar/contracts/grant-schema.md new file mode 100644 index 0000000..ef31fb2 --- /dev/null +++ b/specs/007-notifier-sidecar/contracts/grant-schema.md @@ -0,0 +1,128 @@ +# Contract — Standing Grants (Addendum 001) + +Defines the grant object, the deterministic match predicate grammar, the intake +short-circuit behavior, and the Telegram interaction surface. Companion to +[`openapi.yaml`](./openapi.yaml) (which carries the additive `grant_id` field) +and the data model in `../data-model-addendum-001.md`. + +## Intake short-circuit (server) + +On `POST /v1/approve`, **before** the shutdown/health gate and `reserve()`: + +1. If grants are disabled or globally paused → skip to the normal pipeline. +2. Else evaluate `GrantStore.match(request, now)`. +3. On a match → respond `200` immediately with `ApprovalResponse`: + - `decision: "allow"`, `responder: "rule:{grant_id}"`, + `reason: "auto-approved via standing grant"`, `grant_id: "{id}"`, + `latency_ms` measured, `decided_at` now. + - No notification is sent; no pending slot is reserved. + - A structural audit line is logged; the grant's `uses_count` increments. +4. On no match → the parent v1 pipeline runs unchanged (clamp → reserve → send → + await → 200/408/409/503). + +**Fail-closed**: any error evaluating a grant, an expired/revoked grant, a +scope/predicate mismatch, or a missing request field the rule keys on ⇒ **no +match** ⇒ the human is prompted. + +## Grant object + +```json +{ + "grant_id": "uuid", + "created_at": "2026-06-01T14:00:00Z", + "created_by": "telegram:paulofallon", + "expires_at": "2026-06-01T22:00:00Z", + "source_approval_id": "uuid", + "scope": { "type": "project", "value": "myproj" }, + "eligible": true, + "predicate": { "kind": "command", "command": "git", + "args": ["push"], "args_match": "prefix" }, + "uses_count": 0, + "last_used_at": null +} +``` + +`expires_at = created_at + grants.default_ttl_seconds` (default 8h). No +indefinite grants in v1. `eligible` is always `true` in v1 (reserved for a future +ineligibility denylist). + +## Predicate grammar (deterministic match) + +A request matches iff the grant is **active** (`now < expires_at`, not revoked), +its **scope** matches, and its **predicate** matches — all exact/deterministic +(no fuzzy/semantic matching at runtime). + +**Scope match** (exact equality; `global` always; missing field ⇒ no match): + +| scope.type | compares request field | +|------------|------------------------| +| session | `session_id` | +| workspace | `workspace` | +| project | `project` | +| instance | `instance_id` (or configured instance id) | +| global | — (always; only if `allow_global_scope`) | + +**Predicate match** by `kind` (or by `policy_rule_name` if set — broadest rung): + +| kind | fields | rule | +|------|--------|------| +| any (rule rung) | `policy_rule_name` | request `policy_rule_name` equals it | +| command | `command`, `args`, `args_match` | command equal; args by `exact` (equal) / `prefix` (request starts with `args`) / `glob` (positional glob) | +| file | `paths`, `operations` | request path matches a `paths` glob (`{workspace}` expanded) AND request op ∈ `operations` | +| network | `host`, `host_match`, `port` | host `exact`/`suffix` match AND port equal (port omitted ⇒ any) | +| signal | `signal` | signal equal | + +## Telegram surface + +### Approval keyboard (additive third button) + +``` +[✅ Approve] [⏩ Always…] [❌ Deny] +``` +`callback_data`: `approve:{id}`, `always:{id}`, `deny:{id}`. + +### "Always…" picker (second tap) + +On `always:{id}`, the transport computes `GrantStore.propose(request)` +(tightest-first, **at most 4 candidates**), stashes the candidate list keyed by +`{id}`, and renders: + +``` +Always allow: +[] → pick:{id}:0 +[] → pick:{id}:1 +[] → pick:{id}:2 +[Cancel] → pick:{id}:cancel +``` + +Labels include scope, e.g. `git push * · this project`. `callback_data` stays +≤ 64 bytes by indexing into the stashed candidates (predicate never goes in +`callback_data`). + +On `pick:{id}:{index}`: create the grant (TTL applied), resolve the approval +`allow` (+`grant_id`), edit the message to confirm +(`⏩ Always: git push * · myproj · 8h · by @paulofallon`). On `pick:{id}:cancel`: +restore the original Approve/Deny keyboard. Unauthorized chat, unknown id, or +expired picker ⇒ ignored (consistent with parent FR-011/FR-012). Over the +`max_grants` cap ⇒ confirm message reports "grant limit reached" and the approval +is still allowed once (the grant just isn't created). + +### Slash commands (authorized chat only) + +| Command | Effect | +|---------|--------| +| `/rules` | list active grants (id, class label, scope, age, TTL, uses) with inline `[Revoke]` buttons | +| `/revoke ` | revoke a grant; subsequent matches re-prompt | +| `/pause` / `/resume` | global pause/resume of all auto-approval (FR-G10) | + +Listing is **Telegram-only** — there is no bridge HTTP read endpoint and no CLI +lister in v1 (avoids disclosing the auto-approve set to co-located containers). + +## Audit & digest + +- Each auto-approval logs `auto_approved {approval_id, grant_id, kind, summary, + latency_ms}` at INFO — **no secrets, bodies, or workspace paths** (parent + FR-017). +- A periodic digest (default hourly; `digest_interval_seconds: 0` disables) + messages the chat a count summary when there was activity, so standing grants + are never silent (FR-G11). diff --git a/specs/007-notifier-sidecar/contracts/openapi.yaml b/specs/007-notifier-sidecar/contracts/openapi.yaml new file mode 100644 index 0000000..3bd2c68 --- /dev/null +++ b/specs/007-notifier-sidecar/contracts/openapi.yaml @@ -0,0 +1,149 @@ +openapi: 3.1.0 +info: + title: Remo Notifier — agentsh approval wire protocol + version: "1.0.0" + description: > + Durable contract between agentsh (or any future approval emitter) and the + notifier sidecar. The notifier listens on 0.0.0.0:18181 inside its container; + the host binds it to the Docker bridge address (e.g. 172.17.0.1:18181), + reachable only by co-located devcontainers. No transport encryption and no + caller authentication in v1 (see spec FR-021). Fail-secure: no outcome other + than an explicit authorized human Approve ever yields "allow". +servers: + - url: http://172.17.0.1:18181 + description: Host Docker-bridge binding (example) +paths: + /v1/approve: + post: + summary: Submit an approval request and block until a decision exists + description: > + The connection is held open until a human decides, the timeout fires, or + the service shuts down. Exactly one response is returned. + operationId: approve + requestBody: + required: true + content: + application/json: + schema: { $ref: "#/components/schemas/ApprovalRequest" } + responses: + "200": + description: > + A decision was made (allow or deny) by an authorized human, OR the + request matched an active human-created standing grant and was + auto-approved (Addendum 001). Auto-approvals set responder to + "rule:{grant_id}" and include grant_id. See contracts/grant-schema.md. + content: + application/json: + schema: { $ref: "#/components/schemas/ApprovalResponse" } + "400": + description: Schema validation failure (unknown field, bad type, bad UUID). + content: + application/json: + schema: { $ref: "#/components/schemas/ErrorResponse" } + "408": + description: > + No human responded within the (clamped) timeout. Fail-secure: body is + a full ApprovalResponse with decision=deny, reason="timeout". + content: + application/json: + schema: { $ref: "#/components/schemas/ApprovalResponse" } + "409": + description: > + The supplied approval_id matches an approval already pending (FR-003a). + The original pending approval is left running; no second notification. + content: + application/json: + schema: { $ref: "#/components/schemas/ErrorResponse" } + "503": + description: > + Service unavailable — shutting down, transport unreachable, no + human-side configuration loaded, at max pending-approval capacity + (FR-034), or the notification for THIS request failed to send + (FR-010a). No pending slot is held in the send-failure case. + content: + application/json: + schema: { $ref: "#/components/schemas/ErrorResponse" } + /v1/health: + get: + summary: Liveness probe (no auth) + operationId: health + responses: + "200": + description: Service is up. + content: + application/json: + schema: { $ref: "#/components/schemas/HealthResponse" } +components: + schemas: + Operation: + type: object + additionalProperties: false + required: [kind] + properties: + kind: { type: string, enum: [command, file, network, signal] } + command: { type: string } + args: { type: array, items: { type: string }, default: [] } + path: { type: string } + remote_host: { type: string } + remote_port: { type: integer, minimum: 1, maximum: 65535 } + context: { type: string, enum: [direct, nested], default: direct } + depth: { type: integer, minimum: 0, default: 0 } + ApprovalRequest: + type: object + additionalProperties: false + required: [operation, policy_rule_name, policy_message] + properties: + approval_id: + type: string + format: uuid + description: Optional; server generates if absent. Duplicate-of-pending → 409. + session_id: { type: string, description: Opaque agentsh session id. } + operation: { $ref: "#/components/schemas/Operation" } + policy_rule_name: { type: string } + policy_message: { type: string } + workspace: { type: string, description: DEBUG-log only; never logged at INFO+. } + instance_id: { type: string, description: Falls back to configured instance id. } + project: { type: string } + timeout_seconds: + type: integer + minimum: 1 + description: Clamped to [1, max_timeout_seconds]; defaults to default_timeout_seconds. + submitted_at: { type: string, format: date-time } + ApprovalResponse: + type: object + required: [approval_id, decision, responder, reason, decided_at, latency_ms] + properties: + approval_id: { type: string } + decision: { type: string, enum: [allow, deny] } + responder: + type: string + description: > + e.g. "telegram:paulofallon", "system:timeout", or "rule:{grant_id}" + for a standing-grant auto-approval (Addendum 001). + reason: { type: string, description: May be empty; "timeout" on 408; "auto-approved via standing grant" on a grant match. } + decided_at: { type: string, format: date-time } + latency_ms: { type: integer, minimum: 0 } + grant_id: + type: string + description: > + Present when this response was auto-approved by a standing grant, or + when the response created a grant (the "Always" tap). Optional; + backward-compatible (Addendum 001). + HealthResponse: + type: object + required: [status, version, transport, uptime_seconds, pending_approvals] + properties: + status: { type: string, example: ok } + version: { type: string } + transport: { type: string, example: telegram } + uptime_seconds: { type: integer, minimum: 0 } + pending_approvals: { type: integer, minimum: 0 } + ErrorResponse: + type: object + required: [error] + properties: + error: + type: string + enum: [validation_error, duplicate_approval_id, unavailable] + detail: { type: string, description: Human-readable; never contains secrets. } + approval_id: { type: string, description: Present on 409. } diff --git a/specs/007-notifier-sidecar/contracts/telegram-message.md b/specs/007-notifier-sidecar/contracts/telegram-message.md new file mode 100644 index 0000000..eecf098 --- /dev/null +++ b/specs/007-notifier-sidecar/contracts/telegram-message.md @@ -0,0 +1,56 @@ +# Contract: Telegram message & callback + +Defines the exact human-facing surface for the Telegram transport. Parse mode: `MarkdownV2` (configurable). All dynamic values MUST be MarkdownV2-escaped before insertion (Telegram requires escaping `_ * [ ] ( ) ~ \` > # + - = | { } . !`). + +## Outgoing approval message + +``` +🔐 Approval requested + +*Project:* {project} +*Operation:* {operation.kind}: {operation.command} {operation.args} +*Rule:* {policy_rule_name} +*Message:* {policy_message} +*Instance:* {instance_id} + +Decide within {timeout_seconds // 60} minutes. +``` + +Field rendering: +- `{operation.args}` rendered space-joined; long arg lists may be truncated with an ellipsis for readability (the full args are never required in the message — agentsh holds ground truth). +- Missing optional fields (`project`, `command`) render as a placeholder (`—`) rather than the literal `None`. +- `timeout_seconds` is the **effective** (clamped) timeout; if `< 60`, show seconds instead of minutes. + +## Inline keyboard + +Two buttons on one row: + +| Button text | `callback_data` | +|-------------|-----------------| +| ✅ Approve | `approve:{approval_id}` | +| ❌ Deny | `deny:{approval_id}` | + +`callback_data` MUST stay ≤ 64 bytes (Telegram limit); a UUID approval_id (36 chars) + verb fits. + +## Callback handling + +1. Reject if `callback_query.message.chat.id != authorized_chat_id` → answer the callback quietly, take no action (FR-011). +2. Parse `verb:approval_id`. Unknown verb → ignore. +3. If `approval_id` is not currently pending → answer "already decided / expired", no state change (FR-012). +4. Else resolve: `allow` for `approve`, `deny` for `deny`; responder = `telegram:{username or user_id}`; reason empty. +5. `answerCallbackQuery` to clear the client spinner. + +## Message edits on terminal outcome (FR-013) + +| Outcome | Edited message suffix (replaces buttons) | +|---------|------------------------------------------| +| Approved | `✅ Approved by @{user} at {HH:MM}` | +| Denied | `❌ Denied by @{user} at {HH:MM}` | +| Timeout | `⌛ Timed out — denied (fail-secure)` | +| Cancelled (external/shutdown) | `🚫 Cancelled — resolved elsewhere` | + +Edits remove the inline keyboard so the message can't be re-tapped. Edit failures (e.g. message deleted by the user) are logged at DEBUG and otherwise ignored — they never block resolving the caller. + +## Test command surface (`remo notifier test`) + +Sends an `ApprovalRequest` with `policy_rule_name: "test"`, `policy_message: "This is a test approval — please tap Approve or Deny to confirm wiring."`, `project: "remo-notifier-selftest"`, a short `timeout_seconds` (e.g. 120), and `operation: {kind: command, command: "echo", args: ["wiring-check"]}`. The CLI prints the returned decision (FR-027). diff --git a/specs/007-notifier-sidecar/contracts/transport.md b/specs/007-notifier-sidecar/contracts/transport.md new file mode 100644 index 0000000..419834e --- /dev/null +++ b/specs/007-notifier-sidecar/contracts/transport.md @@ -0,0 +1,81 @@ +# Contract: `NotificationTransport` ABC + +Location: `src/remo_cli/notifier/transports/base.py`. The notifier core depends only on this interface; Telegram is the sole v1 implementation (`transports/telegram.py`). Future Slack/Discord/ntfy backends subclass it without touching intake or registry logic (FR-015). + +```python +from __future__ import annotations + +from abc import ABC, abstractmethod +from collections.abc import Callable + +from remo_cli.notifier.models import ApprovalDecision, ApprovalRequest + + +class NotificationTransport(ABC): + """Delivers approval requests to a human and reports their decision. + + Lifecycle: start() once at app startup; stop() once at shutdown. Between + them, the server calls send_approval_request() per accepted request and may + call cancel() to retract one resolved by other means. + """ + + name: str # e.g. "telegram" — surfaced in /v1/health + + @abstractmethod + async def start(self) -> None: + """Begin receiving human input (e.g. start long-polling). Idempotent.""" + + @abstractmethod + async def stop(self) -> None: + """Stop receiving input and release resources. Idempotent; safe after start failure.""" + + @abstractmethod + async def send_approval_request( + self, + request: ApprovalRequest, + on_response: Callable[[ApprovalDecision], None], + ) -> None: + """Deliver one request to the human. + + MUST raise on delivery failure (the server maps this to 503 and holds no + pending slot — FR-010a). On success, returns once the notification is + delivered; the human's eventual tap invokes on_response(decision) exactly + once with an authorized decision. on_response is a synchronous, loop-safe + callback that resolves the pending approval's Future. + """ + + @abstractmethod + async def cancel(self, approval_id: str) -> None: + """Retract a still-displayed request (e.g. resolved elsewhere or on shutdown). + + Edits/annotates the human-facing message to reflect the non-human outcome. + Idempotent and a no-op for unknown ids. + """ + + async def healthy(self) -> bool: + """Optional readiness signal. Default True; transports may override. + + When False, the server answers new /v1/approve calls with 503 (FR-007). + """ + return True +``` + +## Behavioral contract + +| Guarantee | Requirement | +|-----------|-------------| +| `on_response` is called at most once per request, only with an **authorized** human decision | FR-008, FR-011 | +| Unauthorized chats / unknown approval ids are ignored (no `on_response`) | FR-011, FR-012, edge cases | +| `send_approval_request` raises ⇒ server returns 503, no pending slot held | FR-010a | +| `cancel` edits the human-facing message to show the final non-human outcome | FR-013 | +| `start`/`stop` are idempotent and tolerate partial init | Constitution III (idempotent), shutdown edge case | +| `name` reflected verbatim in `/v1/health.transport` | FR-016 | + +## Telegram implementation notes (normative for v1) + +- Built on `telegram.ext.Application`; long-polling via `updater.start_polling()` inside FastAPI lifespan; **never** `run_polling()` and **never** webhook mode (R1, FR-014). +- Message body and inline keyboard exactly as in `telegram-message.md`. +- `callback_data`: `approve:{approval_id}` / `deny:{approval_id}`. +- A `CallbackQueryHandler` checks the originating chat equals `authorized_chat_id` (else ignore, FR-011), parses `verb:approval_id`, and calls `on_response(ApprovalDecision(...))` guarded so a non-pending id is a no-op (FR-012). +- On resolution it edits the original message (`✅ Approved by @user at HH:MM` / `❌ Denied …` / `⌛ Timed out — denied (fail-secure)` / cancelled) (FR-013). +- Bot token read from file at startup, kept in memory only (FR-019); never logged (FR-017). diff --git a/specs/007-notifier-sidecar/data-model-addendum-001.md b/specs/007-notifier-sidecar/data-model-addendum-001.md new file mode 100644 index 0000000..5fe2f27 --- /dev/null +++ b/specs/007-notifier-sidecar/data-model-addendum-001.md @@ -0,0 +1,151 @@ +# Phase 1 Data Model — Addendum 001: Standing Grants + +Pydantic v2 models, `extra="forbid"` where they parse external/config input. All +times aware-UTC internally, RFC3339 on the wire. **Nothing is persisted** — these +describe in-memory state and additive wire fields. New module: +`src/remo_cli/notifier/grants.py`. + +## Enums (extend existing or add) + +- `OperationKind` — reused from `models.py` (`command|file|network|signal`). +- `GrantScopeType` — `session | workspace | project | instance | global`. +- `ArgMatchType` — `exact | prefix | glob`. +- `HostMatchType` — `exact | suffix`. + +## GrantPredicate (`grants.py`) + +The deterministic, inspectable match rule. Only fields relevant to `kind` are +set. Agentsh-rule-shaped (forward-compat). + +| Field | Type | Applies to | Notes | +|-------|------|-----------|-------| +| `kind` | OperationKind | all | must equal the request operation kind | +| `command` | str \| None | command, signal | exact command string | +| `args` | list[str] | command | the reference args | +| `args_match` | ArgMatchType | command | how `args` compares (exact list / prefix / glob) | +| `paths` | list[str] | file | glob list; `{workspace}` placeholder allowed | +| `operations` | list[str] | file | e.g. `read`/`write`/`delete` | +| `host` | str \| None | network | reference host | +| `host_match` | HostMatchType | network | exact or domain-suffix | +| `port` | int \| None | network | 1–65535 | +| `signal` | str \| None | signal | signal name | +| `policy_rule_name` | str \| None | any | broadest rung: match by agentsh rule name | + +**Match semantics** (`predicate.matches(operation, policy_rule_name) -> bool`), +deterministic, fail-closed on anything unexpected: +- If `policy_rule_name` is set on the predicate → match iff request + `policy_rule_name` equals it (kind still must match). This is the broadest rung. +- Else by kind: + - `command`: request kind==command AND command equal AND args satisfy + `args_match` (exact: equal lists; prefix: request args start with `args`; + glob: each pattern matches positionally). + - `file`: kind==file AND request path matches any `paths` glob (after + `{workspace}` expansion) AND request op ∈ `operations`. + - `network`: kind==network AND host matches (`exact` equal / `suffix` + `request_host.endswith(host)`) AND `port` equal (or predicate port None ⇒ any). + - `signal`: kind==signal AND signal equal. +- Any missing/None field the rule keys on, or an unparseable glob → **False**. + +## GrantScope (`grants.py`) + +| Field | Type | Notes | +|-------|------|-------| +| `type` | GrantScopeType | | +| `value` | str | the captured session/workspace/project/instance id; ignored for `global` | + +`scope.matches(request) -> bool`: equality of the corresponding request field +(`session`→`session_id`, etc.); `global`→True; missing field → **False** (FR-G5/RG5). +**Exception:** for `instance` scope, a request with no `instance_id` falls back +to the notifier's configured instance id (the instance is known from config, not +fail-closed); this is the one intentional deviation from the missing-field rule. + +## Grant (`grants.py` + additive wire object) + +| Field | Type | Notes | +|-------|------|-------| +| `grant_id` | str (uuid) | server-generated | +| `created_at` | datetime | | +| `created_by` | str | e.g. `telegram:paulofallon` | +| `expires_at` | datetime | `created_at + default_ttl_seconds` (FR-G8) | +| `source_approval_id` | str | the approval the human tapped Always on | +| `scope` | GrantScope | | +| `predicate` | GrantPredicate | | +| `eligible` | bool | always True in v1 (D2); reserved for future denylist (FR-G12) | +| `uses_count` | int | incremented on each auto-approval | +| `last_used_at` | datetime \| None | | + +`grant.active(now) -> bool`: `not revoked and now < expires_at`. +`grant.matches(request, now) -> bool`: `active(now) and scope.matches(request) +and predicate.matches(request.operation, request.policy_rule_name)`. + +## CandidateGrant (`grants.py`, transient) + +Returned by `propose(request)` to build the Telegram picker. Not persisted, not +on the wire. + +| Field | Type | Notes | +|-------|------|-------| +| `label` | str | human-facing, e.g. `git push * · this project` | +| `predicate` | GrantPredicate | | +| `scope` | GrantScope | | + +## GrantStore (`grants.py`, in-memory) + +State: `dict[str, Grant]` + `asyncio.Lock` + `max_grants` + `paused: bool`. + +| Method | Behaviour | +|--------|-----------| +| `match(request, now) -> Grant \| None` | first active grant whose `matches()` is true; None if paused/disabled/none. Allow-capable — exact, deterministic (FR-G4). | +| `create(grant) -> Grant` | under lock; raise `GrantLimitReached` if at `max_grants`; else store. | +| `propose(request) -> list[CandidateGrant]` | deterministic tightest-first templates (RG2), at most 4 candidates. Pure. | +| `list() -> list[Grant]` | active grants (for `/rules`). | +| `revoke(grant_id) -> bool` | remove; True if existed. | +| `set_paused(bool)` / `paused` | global pause (FR-G10). | +| `sweep(now) -> int` | drop expired; return count (RG4 hygiene). | + +State transitions: + +```text + create() revoke() / expire / restart + (none) ───────────────▶ ACTIVE ───────────────────────────────────────▶ (gone) + │ ▲ + match() hit │ │ uses_count++ , last_used_at=now + └──┘ (stays ACTIVE) + paused=true ⇒ match() returns None for ALL grants (not a state change) +``` + +Invariants: +- `match()` returns a grant only if `active(now)` and scope+predicate match. +- An expired grant is never returned even if the sweeper hasn't run (lazy check). +- `len(store) <= max_grants`. +- No `allow` is produced by the store except via a `match()` hit on a + human-created grant (parent FR-008, evolved). + +## Wire/config additions + +### `ApprovalResponse` (modify `models.py`) +- Add `grant_id: str | None = None`. Present when the response was auto-approved + (`responder = rule:{grant_id}`) or when this response *created* a grant (the + Always tap echoes the new grant id). Backward-compatible (optional). + +### `GrantsConfig` (modify `config.py`, `extra="forbid"`) + +| Key | Type | Default | Notes | +|-----|------|---------|-------| +| `enabled` | bool | true | master switch | +| `default_ttl_seconds` | int ≥ 1 | 28800 | 8h; applied to every grant (FR-G8) | +| `max_grants` | int ≥ 1 | 100 | cap (FR-G9) | +| `allow_global_scope` | bool | true | D2; if false, `global` scope is refused | +| `digest_interval_seconds` | int ≥ 0 | 3600 | 0 disables the digest | + +Add `grants: GrantsConfig = Field(default_factory=GrantsConfig)` to +`NotifierConfig`. + +## Relationships + +- `GrantStore` 1—N `Grant`; each `Grant` 1—1 `GrantPredicate` + `GrantScope`. +- A short-circuited `ApprovalRequest` maps to exactly one matched `Grant` + (echoed as `grant_id`); a non-matched request flows through the parent v1 + pipeline unchanged. +- `propose()` turns one `ApprovalRequest` into N `CandidateGrant`s for the picker; + the human's pick becomes one persisted-in-memory `Grant`. diff --git a/specs/007-notifier-sidecar/data-model.md b/specs/007-notifier-sidecar/data-model.md new file mode 100644 index 0000000..0028493 --- /dev/null +++ b/specs/007-notifier-sidecar/data-model.md @@ -0,0 +1,166 @@ +# Phase 1 Data Model: Notifier Sidecar + +All models are Pydantic v2 (`model_config = ConfigDict(extra="forbid")` for strictness where noted). Times are RFC3339 UTC strings on the wire; internally `datetime` (aware, UTC). No model is persisted — these describe in-flight HTTP payloads, configuration, and the in-memory registry. + +## Wire models (`notifier/models.py`) + +### Operation + +Describes the operation agentsh is asking to approve. All fields optional except `kind`; presence depends on `kind`. + +| Field | Type | Required | Notes | +|-------|------|----------|-------| +| `kind` | enum: `command` \| `file` \| `network` \| `signal` | yes | drives which other fields are meaningful | +| `command` | str | no | e.g. `rm`, `curl` (command/signal ops) | +| `args` | list[str] | no | default `[]` | +| `path` | str | no | file ops | +| `remote_host` | str | no | network ops | +| `remote_port` | int | no | network ops; 1–65535 if present | +| `context` | enum: `direct` \| `nested` | no | default `direct` | +| `depth` | int | no | ≥0; default 0 | + +Validation: `extra="forbid"`. No hard cross-field requirement (agentsh owns semantics); notifier renders whatever is present. + +### ApprovalRequest (POST /v1/approve body) + +| Field | Type | Required | Notes | +|-------|------|----------|-------| +| `approval_id` | str (UUID) | no | server generates if absent (FR-003); if present and already pending → 409 (FR-003a) | +| `session_id` | str | no | opaque to notifier | +| `operation` | Operation | yes | | +| `policy_rule_name` | str | yes | | +| `policy_message` | str | yes | human-readable prompt | +| `workspace` | str | no | filesystem path; DEBUG-log only (FR-017) | +| `instance_id` | str | no | falls back to configured instance id if absent | +| `project` | str | no | devcontainer/project name | +| `timeout_seconds` | int | no | clamped to `[1, max_timeout_seconds]`; defaults to `default_timeout_seconds` (FR-006) | +| `submitted_at` | str (RFC3339) | no | informational | + +Validation: `extra="forbid"` → unknown fields produce 400 (FR-001). `approval_id`, if supplied, must be a valid UUID string. + +### ApprovalDecision (internal resolution value) + +The value a `PendingApproval`'s Future is resolved with. + +| Field | Type | Notes | +|-------|------|-------| +| `decision` | enum: `allow` \| `deny` | `allow` only from authorized human Approve (FR-008) | +| `responder` | str | e.g. `telegram:paulofallon`, or `system:timeout` | +| `reason` | str | may be empty; `"timeout"` on timeout | +| `decided_at` | datetime (UTC) | | + +### ApprovalResponse (POST /v1/approve response body) + +| Field | Type | Notes | +|-------|------|-------| +| `approval_id` | str | echoes the (possibly generated) id (FR-003) | +| `decision` | enum: `allow` \| `deny` | | +| `responder` | str | | +| `reason` | str | | +| `decided_at` | str (RFC3339) | | +| `latency_ms` | int | wall time from intake to resolution | + +### HealthResponse (GET /v1/health body) + +| Field | Type | Notes | +|-------|------|-------| +| `status` | str | `"ok"` | +| `version` | str | the **notifier component** version (e.g. `0.1.0`), sourced from a module constant `remo_cli.notifier.__version__` and matching the image tag `remo_notifier_version` — not the `remo-cli` package version (I2) | +| `transport` | str | active transport name, e.g. `"telegram"` | +| `uptime_seconds` | int | since process start | +| `pending_approvals` | int | current registry size | + +### ErrorResponse (4xx/503 bodies, where not the 408 deny shape) + +| Field | Type | Notes | +|-------|------|-------| +| `error` | str | machine code, e.g. `duplicate_approval_id`, `validation_error`, `unavailable` | +| `detail` | str | human-readable; never contains secrets | +| `approval_id` | str | present on 409 | + +408 body is the special fail-secure shape: `{ "approval_id": "...", "decision": "deny", "reason": "timeout", ... }` (a full `ApprovalResponse`), per FR-005. + +## Configuration models (`notifier/config.py`) + +Loaded from TOML (`--config`, default `/etc/notifier/notifier.toml`). Every model is `extra="forbid"` (FR-018 strict mode → unknown keys raise a clear error). + +```text +NotifierConfig +├── server: ServerConfig +│ ├── listen_host: str = "0.0.0.0" +│ ├── listen_port: int = 18181 # 1–65535 +│ └── log_level: enum(debug|info|warning|error) = "info" +├── approval: ApprovalConfig +│ ├── default_timeout_seconds: int = 300 # ≥1 +│ ├── max_timeout_seconds: int = 1800 # ≥ default +│ └── max_pending_approvals: int = 50 # ≥1 (FR-034) +├── transport: TransportConfig +│ ├── type: enum(telegram) = "telegram" # only telegram in v1 +│ └── telegram: TelegramConfig +│ ├── bot_token_file: str = "/run/secrets/telegram_bot_token" +│ ├── authorized_chat_id: int # required +│ └── message_parse_mode: str = "MarkdownV2" +└── instance: InstanceConfig + └── id: str # shown to humans (FR-020) +``` + +Validation rules: +- `max_timeout_seconds >= default_timeout_seconds` (model validator). +- `transport.type == "telegram"` requires `transport.telegram` present and `authorized_chat_id` set. +- `bot_token_file` must exist and be readable at startup; the token is read then (never stored in config) (FR-019). Empty/missing token → fail fast with a clear message (mirrors role pre-flight, FR-023/Constitution IV). + +## In-memory registry (`notifier/state.py`) + +### PendingApproval + +| Field | Type | Notes | +|-------|------|-------| +| `approval_id` | str | key | +| `request` | ApprovalRequest | the originating request (effective timeout already computed) | +| `future` | `asyncio.Future[ApprovalDecision]` | resolved exactly once | +| `created_at` | datetime (UTC) | for latency + uptime accounting | + +### PendingApprovals (registry) + +State: `dict[str, PendingApproval]` + `asyncio.Lock` + `max_pending` cap. + +Operations (all cap/dup checks under the lock): +- `try_register(request) -> PendingApproval | RegisterError` — returns `DUPLICATE` if id pending (→409), `AT_CAPACITY` if `len == max_pending` (→503), else inserts and returns the entry. **Caller registers only after a successful notification send** (R2/FR-010a) — so the public flow is: capacity/dup *pre-check* under lock to reserve, send, then finalize; on send failure, release the reservation. Implementation reserves the slot+id atomically, then rolls back on send failure to honor both FR-034 and FR-010a. +- `resolve(approval_id, decision)` — if still pending, `future.set_result(decision)` and remove; else no-op (late/duplicate callback edge case). +- `cancel(approval_id, decision)` — transport-driven external resolution; same as resolve plus message edit responsibility lies with the transport. +- `count()` — current size (for health). +- `drain(decision)` — on shutdown, resolve all pending with a non-allow outcome / release callers (edge case: shutdown). + +### State transitions + +```text + try_register (slot reserved, id locked) + (none) ─────────────────────────────────────────▶ RESERVED + │ + send notification │ + ┌──────────────────────────────┬──────────┘ + send fails send ok + │ │ + ▼ ▼ + release slot → 503 (FR-010a) PENDING ──────────────┐ + │ │ + human Approve/Deny ──────────┤ │ timeout + │ ▼ + │ RESOLVED(deny,"timeout") → 408 + cancel(external) ────────────┤ + │ shutdown + ▼ ▼ + RESOLVED(decision) → 200 drained → conn drop/503 +``` + +Invariants: +- A Future is resolved exactly once; all post-resolution callbacks are no-ops. +- `allow` enters the system only via the authorized-human Approve edge (FR-008, SC-005). +- `len(registry) <= max_pending` always (FR-034). +- At most one PENDING entry per `approval_id` (FR-003a). + +## Relationships + +- `ApprovalRequest` 1—1 `PendingApproval` (while in flight) 1—1 `ApprovalResponse` (at resolution). +- `NotifierConfig.transport` selects exactly one `Transport` instance for the process lifetime. +- `Transport` delivers a message per `PendingApproval` and reports decisions back through the registry's `resolve`/`cancel`. diff --git a/specs/007-notifier-sidecar/plan-addendum-001.md b/specs/007-notifier-sidecar/plan-addendum-001.md new file mode 100644 index 0000000..d0667c9 --- /dev/null +++ b/specs/007-notifier-sidecar/plan-addendum-001.md @@ -0,0 +1,120 @@ +# Implementation Plan — Addendum 001: Standing Grants ("Always") + +**Branch**: `007-notifier-sidecar` | **Date**: 2026-06-01 +**Spec**: [`addendum-001-standing-grants.md`](./addendum-001-standing-grants.md) +**Parent plan**: [`plan.md`](./plan.md) (shipped v1 notifier) + +**Note**: This plans the *addendum* only. The base notifier (server, registry, +Telegram transport, config, CLI, role, tests) is implemented and merged on this +branch; this layers standing-grant auto-approval on top. It does not overwrite +the base artifacts. + +## Summary + +Add human-authored **standing grants** so a third Telegram choice — **Always** — +auto-approves a *class* of operation without a round-trip. A new +`src/remo_cli/notifier/grants.py` holds the `Grant`/predicate/scope models, an +in-memory `GrantStore` (TTL-bounded, capped, fail-closed), a **deterministic +per-operation matcher**, and a **candidate-generalization** proposer. `server.py` +gains an intake short-circuit (`/v1/approve` checks the store before +reserve/notify). The Telegram transport gains the `[✅ Approve] [⏩ Always…] +[❌ Deny]` flow with a scope/granularity picker, plus `/rules`, `/revoke`, +`/pause` command handlers and an auto-approval digest. Config grows a `[grants]` +block; the `remo_notifier` role's config template + defaults grow matching +variables. All grant state is in-memory (no persistence); everything stays +fail-secure — nothing is auto-`allow`ed except by an active, unexpired, +unrevoked, human-created grant whose deterministic predicate the request +satisfies. + +## Technical Context + +**Language/Version**: Python 3.11+ (service container 3.13) — unchanged. +**Primary Dependencies**: unchanged (`[notifier]` extra: FastAPI, uvicorn, +pydantic, python-telegram-bot, structlog). **No new runtime dependencies.** +**Storage**: None — grants are in-memory in a `GrantStore`, never persisted +(extends parent FR-009; restart re-prompts, fail-closed). +**Testing**: pytest + pytest-asyncio + httpx (existing). New `tests/notifier/ +test_grants.py`; extensions to `test_server.py` and `test_telegram.py`. +**Target Platform**: unchanged (Linux container; operator CLI cross-platform). +**Project Type**: single project — additive within `src/remo_cli/notifier/`. +**Performance Goals**: auto-approve match returns `allow` in < 500 ms (SC-G1) — +trivially met by an in-memory dict/predicate match. +**Constraints**: deterministic exact matcher (allow-capable — highest +criticality, FR-G4); narrow default scope (FR-G7); single global TTL default 8h, +no indefinite grants (FR-G8); max-grants cap (FR-G9); fail-closed on miss / +ambiguity / restart; no secrets in grant logs (FR-017); listing/revoke +Telegram-only, no bridge read endpoint (OQ4). +**Scale/Scope**: one authorized chat; ≤ `max_grants` (default 100) active grants +per instance; ~250–400 LOC Python + ~15 LOC Ansible (config block) + tests/docs. + +## Constitution Check + +*GATE: must pass before Phase 0. Re-check after Phase 1.* + +| Principle | Gate | Status | +|-----------|------|--------| +| I. Defensive Variable Access (Ansible) | Only the role's `notifier.toml.j2` + `defaults/main.yml` change (a `[grants]` block + vars); no registered-variable access added | PASS — no new `.rc`/`.stdout` usage | +| II. Test All Conditional Paths | Matcher branches (match / no-match / expired / revoked / paused / at-capacity / scope-mismatch) and the short-circuit vs miss paths are each tested both ways | PASS — enumerated in tasks/tests | +| III. Idempotent by Default | The role change is a templated config block (handler-restart on change); re-runs are no-ops. Runtime grant state is in-memory, not config | PASS | +| IV. Fail Fast with Clear Messages | `GrantsConfig` is strict (`extra="forbid"`); invalid grant config fails startup with a clear message, like the rest of the config | PASS | +| V. Documentation Reflects Reality | wire-protocol.md (short-circuit + `grant_id`), config-schema.md (`[grants]`), notifier README, top-level README updated alongside code | PASS | + +**Fail-secure invariant evolution (explicit):** parent FR-008 ("`allow` only from +an explicit human tap") becomes "`allow` only from an explicit human tap **or** a +matching active human-created standing grant." Still human-authored; every +non-matching path still prompts or fails closed. This is a deliberate, +documented relaxation, not a violation. No other constitution gate is affected. + +**Complexity Tracking**: empty (no deviations to justify). + +Post-Phase-1 re-check: design keeps grants additive under `notifier/`, the +matcher deterministic and transport-agnostic (enforcement in `server.py`), grant +UI confined to the Telegram transport, and the store in-memory. **Still PASS.** + +## Project Structure (delta) + +```text +src/remo_cli/notifier/ +├── grants.py # NEW — Grant/predicate/scope models, GrantStore, +│ # deterministic matcher, candidate proposer +├── server.py # MODIFIED — intake short-circuit; own GrantStore; +│ # inject into transport; TTL sweep + digest +│ # in lifespan +├── config.py # MODIFIED — GrantsConfig (enabled/ttl/max/global) +├── models.py # MODIFIED — ApprovalResponse gains optional grant_id +└── transports/ + ├── base.py # MODIFIED (minimal) — transport may accept a grant + │ # service; grant UI is implementation-specific + └── telegram.py # MODIFIED — Always button + picker callback flow, + # grant creation, /rules /revoke /pause, + # digest sender + +ansible/roles/remo_notifier/ +├── defaults/main.yml # MODIFIED — grant vars (enabled/ttl/max/global) +└── templates/notifier.toml.j2 # MODIFIED — [grants] block + +tests/notifier/ +├── test_grants.py # NEW — store, matcher, candidates, TTL, cap, revoke, +│ # pause, scope, fail-closed, concurrency +├── test_server.py # MODIFIED — short-circuit allow; miss→existing flow +└── test_telegram.py # MODIFIED — Always flow, picker callback_data, + # grant creation, /rules /revoke /pause + +src/remo_cli/notifier/docs/ # MODIFIED — wire-protocol.md, config-schema.md +README.md # MODIFIED — notifier "always" note +specs/007-notifier-sidecar/contracts/ +├── openapi.yaml # MODIFIED — ApprovalResponse.grant_id; 200 may be auto +└── grant-schema.md # NEW — Grant/predicate grammar, matching, callback_data +``` + +**Structure Decision**: Additive within the existing notifier package. Critical +separation: **enforcement (matching) lives in `server.py`/`grants.py` +(transport-agnostic); the grant UI (button, picker, slash-commands) lives in the +Telegram transport.** The `GrantStore` is owned by the app (constructed in +`create_app`) and injected into the transport so the transport can propose/create +grants and serve `/rules`/`/revoke`/`/pause`, while the server alone performs the +allow-capable match. No CLI changes (OQ4); no new top-level integration points. + +## Complexity Tracking + +> No constitution violations — section intentionally empty. diff --git a/specs/007-notifier-sidecar/plan.md b/specs/007-notifier-sidecar/plan.md new file mode 100644 index 0000000..2cc9651 --- /dev/null +++ b/specs/007-notifier-sidecar/plan.md @@ -0,0 +1,124 @@ +# Implementation Plan: Notifier Sidecar — Telegram approval bridge for agentsh + +**Branch**: `007-notifier-sidecar` | **Date**: 2026-05-31 | **Spec**: [spec.md](./spec.md) +**Input**: Feature specification from `/specs/007-notifier-sidecar/spec.md` + +## Summary + +Add a self-contained notifier service to the existing `remo-cli` package: a long-running FastAPI daemon that runs in a hardened OCI container on each remo-provisioned host. It accepts agentsh approval requests over HTTP, delivers them to one authorized human via a Telegram bot (long-polling, no public URL), and returns the human's allow/deny decision synchronously — failing secure (deny) on timeout, shutdown, send failure, or capacity exhaustion. The service holds no persistent state. + +Technical approach: a new `src/remo_cli/notifier/` sub-package (a fourth peer of `cli/`, `providers/`, `core/`) carries the server, config, in-memory pending-approval registry, wire-protocol models, and a pluggable `NotificationTransport` ABC with a single Telegram implementation. The Telegram `Application` runs long-polling inside the FastAPI lifespan on the same asyncio loop. A new `remo_notifier` Ansible role (depends-on `docker`) builds the image on the host from a build context shipped inside the wheel, renders config + secret, and installs a systemd unit that runs `docker run` bound to the Docker bridge address. New `remo notifier {deploy,status,logs,test,restart}` CLI subcommands drive deployment and day-2 ops using the existing host-resolution, picker, SSH, and ansible-runner helpers. The notifier's runtime deps live behind a `[notifier]` optional extra so the laptop install is unaffected. + +## Technical Context + +**Language/Version**: Python 3.11+ (package `requires-python = ">=3.11"`); service container runs Python 3.13-slim +**Primary Dependencies**: Service (new `[notifier]` extra): FastAPI ≥0.115, uvicorn[standard] ≥0.32, pydantic ≥2.9, python-telegram-bot ≥21.6, structlog ≥24.4, tomli (py<3.11 only). CLI side: Click ≥8.1 (existing), no new laptop runtime deps. Build: hatchling (existing), uv (in-container). Ansible: new `community.docker` collection. +**Storage**: None. All approval state is in-memory in a `PendingApprovals` registry; never persisted (FR-009). +**Testing**: pytest (existing) + new dev deps pytest-asyncio and httpx (FastAPI `TestClient`/`AsyncClient`). Tests under `tests/notifier/`. +**Target Platform**: Service: Linux/amd64 OCI container on Ubuntu 24.04 hosts. Operator CLI: cross-platform (existing remo install). +**Project Type**: Single project — Python package (`src/remo_cli/`) + Ansible (`ansible/`), matching existing repo layout. +**Performance Goals**: Decision delivered to caller within 5 s of a human tap (SC-001); timeout-deny returned within ~1 s of the request deadline (SC-002); health check answers within 5 s of container start (SC-003). +**Constraints**: Container image <250 MB (SC/AC-2); listener bound to the Docker bridge address only, no TLS, no caller auth (FR-021); no secrets at INFO+ log level (FR-017); fail-secure — never "allow" except an explicit human tap (FR-008); no persistence (FR-009); default max 50 concurrent pending approvals (FR-034). +**Scale/Scope**: One authorized Telegram chat per instance; ~600–900 LOC Python, ~150 LOC Ansible/Jinja, ~50 LOC CLI; configurable cap (default 50) on concurrent pending approvals per instance. + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +The constitution (v1.0.0) is centered on Ansible quality plus fail-fast and docs-reflect-reality. Evaluating the new `remo_notifier` role and surrounding work: + +| Principle | Gate | Status | +|-----------|------|--------| +| I. Defensive Variable Access (Ansible) | All registered-var attribute access in `remo_notifier` role uses `\| default()`; `.rc`/`.stdout` never bare; `when:` guards use `is defined` / `\| default()` | PASS — committed; pre-commit grep (`grep -r '\.rc ==' ansible/`, `grep -r '\.stdout' ansible/`) in the task list | +| II. Test All Conditional Paths | Role toggles (`configure_remo_notifier`, `remo_notifier_build_from_source`) and Python fail-secure branches (timeout, send-failure, capacity, shutdown, duplicate-id) are each tested both ways | PASS — Python branches covered by T010/T011 (incl. timeout-clamp); role toggle/pull paths covered by **T031a** (or explicitly deferred there) | +| III. Idempotent by Default | Role re-run yields identical state: config/secret templates use `changed_when`/handlers; image build is conditional; `docker run` via systemd is declarative; health-wait is a read-only check | PASS — verified by the rerun check in **T045a** | +| IV. Fail Fast with Clear Messages | Pre-flight `assert` for empty bot token / chat id with actionable `fail_msg` (FR-023); config strict-validation rejects unknown keys with a clear error (FR-018); deploy fails if health never comes up (FR-025) | PASS | +| V. Documentation Reflects Reality | wire-protocol.md + config-schema.md + notifier README + top-level README "Notifier" section land with the code; examples are runnable (FR/req 10) | PASS | + +No violations. **Complexity Tracking** is empty (no deviations to justify). + +Post-Phase-1 re-check: design keeps the notifier strictly additive (no edits to `docker`/`devcontainers`/provider roles per spec constraint), the role self-contained with a single `meta` dependency on `docker`, and the laptop install free of service deps — all consistent with the gates above. **Still PASS.** + +## Project Structure + +### Documentation (this feature) + +```text +specs/007-notifier-sidecar/ +├── plan.md # This file +├── spec.md # Feature spec (already written + clarified) +├── research.md # Phase 0 output +├── data-model.md # Phase 1 output +├── quickstart.md # Phase 1 output +├── contracts/ # Phase 1 output +│ ├── openapi.yaml # HTTP wire protocol (POST /v1/approve, GET /v1/health) +│ ├── transport.md # NotificationTransport ABC contract +│ └── telegram-message.md # Telegram message + callback_data contract +├── checklists/ +│ └── requirements.md # Spec quality checklist (done) +└── tasks.md # /speckit.tasks output (NOT created here) +``` + +### Source Code (repository root) + +```text +src/remo_cli/ +├── notifier/ # NEW — fourth layer, peer of cli/ providers/ core/ +│ ├── __init__.py +│ ├── cli.py # `remo-notifier serve` entry point (Click or argparse) +│ ├── server.py # FastAPI app, lifespan (start/stop transport), routes +│ ├── config.py # Pydantic config models + strict TOML loader +│ ├── state.py # PendingApprovals registry (asyncio, cap-aware) +│ ├── models.py # ApprovalRequest/Response/Operation/Decision (Pydantic) +│ ├── logging_setup.py # structlog config; secret-safe processors +│ ├── transports/ +│ │ ├── __init__.py +│ │ ├── base.py # NotificationTransport ABC +│ │ └── telegram.py # Telegram Application (long-polling) implementation +│ ├── docs/ +│ │ ├── wire-protocol.md +│ │ └── config-schema.md +│ └── README.md +├── cli/ +│ └── notifier.py # NEW — `remo notifier {deploy,status,logs,test,restart}` +├── cli/main.py # MODIFIED — register the `notifier` group +├── core/ providers/ models/ # UNCHANGED (constraint: no notifier code here) +└── ansible/ # force-included into wheel (existing mechanism) + +ansible/ # (mirrors into wheel at remo_cli/ansible) +├── roles/remo_notifier/ # NEW role +│ ├── tasks/main.yml +│ ├── defaults/main.yml +│ ├── handlers/main.yml +│ ├── meta/main.yml # dependencies: [docker] +│ └── templates/ +│ ├── remo-notifier.service.j2 +│ └── notifier.toml.j2 +├── notifier_deploy.yml # NEW playbook: apply remo_notifier to a target host +├── tasks/configure_dev_tools.yml# MODIFIED — include remo_notifier when toggled +├── group_vars/all.yml # MODIFIED — notifier vars (env-backed secrets) +└── requirements.yml # MODIFIED — add community.docker collection + +notifier/ +└── Dockerfile # NEW — multi-stage build (repo root, NOT under src/) + +tests/notifier/ # NEW — mirrors the package +├── __init__.py +├── conftest.py # fixtures: fake transport, test config, async loop +├── test_models.py +├── test_config.py +├── test_state.py # registry: register/cancel/timeout/resolve/cap/dup-id +├── test_server.py # all status codes; TestClient + AsyncClient +├── test_telegram.py # Bot mock; message/keyboard/callback/edit/cancel +└── test_cli_notifier.py # subcommands with mocked SSH + fake host registry + +pyproject.toml # MODIFIED — [notifier] extra, remo-notifier script, + # dev deps (pytest-asyncio, httpx), force-include build ctx +README.md # MODIFIED — "Notifier" + "Notifier setup" sections +``` + +**Structure Decision**: Single-project layout, matching the existing repo. The notifier is an additive fourth peer package under `src/remo_cli/notifier/`; the only edits to existing files are pure additions/registrations (`cli/main.py`, `pyproject.toml`, `group_vars/all.yml`, `tasks/configure_dev_tools.yml`, `requirements.yml`) so no existing command's behavior changes (FR-032). The Ansible role is self-contained with one `meta` dependency on `docker`; no existing role is modified (spec constraint). + +## Complexity Tracking + +> No constitution violations — section intentionally empty. diff --git a/specs/007-notifier-sidecar/quickstart.md b/specs/007-notifier-sidecar/quickstart.md new file mode 100644 index 0000000..cb5a2fa --- /dev/null +++ b/specs/007-notifier-sidecar/quickstart.md @@ -0,0 +1,170 @@ +# Quickstart: Notifier Sidecar + +Two audiences: an **operator** standing the notifier up on a host, and a **developer** working on the notifier code. Both assume the existing remo dev setup (`uv sync`, Python ≥3.11). + +## Operator: zero → working approval loop + +### 1. Create a Telegram bot +- In Telegram, message `@BotFather`, run `/newbot`, follow the prompts. Save the bot token it returns. + +### 2. Find your chat id +- Message `@userinfobot` from your own account; it replies with your numeric user id. That is your `authorized_chat_id`. + +### 3. Message your new bot once +- Open a chat with your bot and send any message. A bot cannot DM you until you've initiated a chat. + +### 4. Export credentials on the laptop (where `remo` runs) +```bash +export REMO_NOTIFIER_TELEGRAM_BOT_TOKEN="12345:ABC...your-token" +export REMO_NOTIFIER_TELEGRAM_CHAT_ID="987654321" +``` + +### 5. Deploy to a host +```bash +remo notifier deploy # omit to fuzzy-pick from known hosts +``` +This applies the `remo_notifier` Ansible role: pre-flights the credentials, renders `/etc/notifier/notifier.toml`, writes the token to `/etc/notifier/secrets/telegram_bot_token` (0400), builds the image on the host, installs and starts `remo-notifier.service`, and waits for `/v1/health` to return 200. + +### 6. Verify end-to-end +```bash +remo notifier test +``` +You should get a Telegram message within ~2 s; tapping **Approve** prints `decision: allow`, **Deny** prints `decision: deny`. + +### Day-2 +```bash +remo notifier status # /v1/health JSON +remo notifier logs --follow # journalctl -u remo-notifier.service -f +remo notifier restart # systemctl restart +remo notifier deploy --rebuild # force image rebuild +``` + +### Rotate the bot token +Update the secret on the host and restart: +```bash +remo notifier restart # after rewriting /etc/notifier/secrets/telegram_bot_token +``` +(The service also re-reads the token on `SIGHUP`.) + +## Developer: run and test locally + +### Install with the notifier extra +```bash +uv pip install -e ".[notifier]" # adds FastAPI, uvicorn, pydantic, python-telegram-bot, structlog +remo-notifier --help +``` + +### Run the server against a local config +```toml +# /tmp/notifier.toml +[server] +listen_host = "127.0.0.1" +listen_port = 18181 +log_level = "debug" + +[approval] +default_timeout_seconds = 300 +max_timeout_seconds = 1800 +max_pending_approvals = 50 + +[transport] +type = "telegram" + +[transport.telegram] +bot_token_file = "/tmp/telegram_token" # echo your token into this file +authorized_chat_id = 987654321 +message_parse_mode = "MarkdownV2" + +[instance] +id = "dev-local" +``` +```bash +printf '%s' "$REMO_NOTIFIER_TELEGRAM_BOT_TOKEN" > /tmp/telegram_token +chmod 0400 /tmp/telegram_token +remo-notifier serve --config /tmp/notifier.toml +curl -s http://127.0.0.1:18181/v1/health | jq +``` + +### Exercise the wire protocol +```bash +# Times out → 408 fail-secure deny after ~5s +curl -i -X POST http://127.0.0.1:18181/v1/approve \ + -H 'content-type: application/json' \ + -d '{"operation":{"kind":"command","command":"rm","args":["-rf","/tmp/x"]}, + "policy_rule_name":"demo","policy_message":"approve rm?","timeout_seconds":5}' +``` + +### Build & size-check the image +```bash +docker build -t remo-notifier:0.1.0 -f notifier/Dockerfile . +docker images remo-notifier:0.1.0 # expect < 250 MB +docker run --rm -p 18181:18181 \ + -v /tmp/notifier.toml:/etc/notifier/notifier.toml:ro \ + -v /tmp/telegram_token:/run/secrets/telegram_bot_token:ro \ + remo-notifier:0.1.0 +``` + +### Tests, lint, types +```bash +uv run pytest tests/notifier/ --cov=remo_cli.notifier # target > 85% +uv run ruff check src/remo_cli/notifier/ +uv run mypy src/remo_cli/notifier/ +# Ansible safety (constitution): expect no bare attribute access +grep -r '\.rc ==' ansible/roles/remo_notifier/ ; grep -r '\.stdout' ansible/roles/remo_notifier/ +``` + +## Acceptance smoke (maps to spec Success Criteria) + +| Check | Spec | +|-------|------| +| Tap Approve → caller gets `allow` < 5 s | SC-001 | +| No tap, 5 s timeout → 408 `{decision: deny, reason: timeout}` in 5–6 s | SC-002, AC-7 | +| `deploy` → service `active (running)`, health 200 < 5 s | SC-003, AC-3 | +| Zero → working via documented steps + deploy + test | SC-004 | +| Only human Approve yields `allow` (timeout/400/503/shutdown never do) | SC-005 | +| No token/body/workspace at INFO+ logs | SC-006 | +| Existing `remo` commands unchanged; laptop install lacks notifier deps | SC-007 | +| Wire protocol fully documented (contracts/) | SC-008 | +| Image < 250 MB; health 200 < 5 s in container | AC-2 | +| `pytest tests/notifier/` > 85% coverage | AC-8 | +| `ruff` + `mypy` clean on notifier package | AC-9 | + +## Standing grants — "Always" (Addendum 001) + +### Try it locally + +With `remo-notifier serve` running and a real bot: + +```bash +# Send a request; in Telegram tap [⏩ Always…], then pick a generalization+scope. +curl -i -X POST http://127.0.0.1:18181/v1/approve -H 'content-type: application/json' \ + -d '{"operation":{"kind":"command","command":"git","args":["push","origin","main"]}, + "policy_rule_name":"vcs-push","policy_message":"allow git push?","project":"demo"}' + +# Repeat the same request — it should now return 200 allow instantly, no Telegram, +# with responder "rule:" and a grant_id field. +``` + +Manage grants from Telegram: `/rules` (list + revoke buttons), `/revoke `, +`/pause` and `/resume`. + +### Config (notifier.toml) + +```toml +[grants] +enabled = true +default_ttl_seconds = 28800 # 8h; every grant expires (no indefinite grants) +max_grants = 100 +allow_global_scope = true +digest_interval_seconds = 3600 # 0 disables the auto-approval digest +``` + +### Acceptance smoke (Addendum) + +| Check | Spec | +|-------|------| +| After "Always", a matching request returns `allow` < 500 ms, no Telegram | SC-G1 | +| Only an active/unexpired/unrevoked human grant ever auto-approves; else prompt/fail-closed | SC-G2 | +| `/rules` lists, `/revoke` and `/pause` take effect immediately | SC-G3 | +| Every auto-approval is auditable by `grant_id`, no secrets in the record | SC-G4 | +| Restart clears all grants → re-prompts (fail-closed) | SC-G5 | diff --git a/specs/007-notifier-sidecar/research-addendum-001.md b/specs/007-notifier-sidecar/research-addendum-001.md new file mode 100644 index 0000000..dc5d886 --- /dev/null +++ b/specs/007-notifier-sidecar/research-addendum-001.md @@ -0,0 +1,152 @@ +# Phase 0 Research — Addendum 001: Standing Grants + +Product-level questions were settled in `/speckit.clarify` (see the addendum's +Clarifications: no bundling, offer-always, single 8h TTL, Telegram-only listing). +This document resolves the remaining **implementation** decisions. No +`NEEDS CLARIFICATION` markers remain except the external OQ1 (agentsh signing), +which only gates the *future* promotion path, not v1. + +## RG1. Where the GrantStore lives and how the transport reaches it + +**Decision**: `GrantStore` is instantiated in `create_app()` (one per process, +alongside the `PendingApprovals` registry). The **server** performs the +allow-capable match at intake. The store is **injected into the transport** +(constructor arg) so the Telegram transport can (a) ask `grants.propose(request)` +for picker candidates, (b) call `grants.create(grant)` on selection, and (c) back +`/rules`/`/revoke`/`/pause`. Matching (enforcement) never happens in the +transport. + +**Rationale**: Keeps the security-critical match in one transport-agnostic place +(`server.py`), while the grant UI is a Telegram concern. One store owner avoids +split state. Mirrors how the registry is already owned by `create_app`. + +**Alternatives**: store inside the transport (rejected — couples enforcement to a +specific transport, and the server needs it for the short-circuit); global +singleton (rejected — breaks test isolation, which relies on per-app instances). + +## RG2. Candidate-generalization templates (the proposer) + +**Decision**: `grants.propose(request) -> list[CandidateGrant]` returns an +ordered, **tightest-first** list of `(predicate, scope, label)` built by +deterministic templates per `operation.kind`: + +- `command`: ① exact command+args → ② command + arg-prefix (`git push *`) → + ③ command only (`git *`) → ④ (broadest) `policy_rule_name` predicate. +- `file`: ① exact path+op → ② path's directory glob + op (`{workspace}/sub/** · + delete`) → ③ workspace-rooted glob + op → ④ `policy_rule_name`. +- `network`: ① exact host+port → ② host **suffix** + port (`*.github.com:443`) → + ③ `policy_rule_name`. +- `signal`: ① exact signal+target → ② `policy_rule_name`. + +Each candidate is paired with the **narrowest scope covering the request** by +default (FR-G7); the picker also offers one widened scope rung. The proposer is +pure (no I/O), so it is unit-testable in isolation. + +**Rationale**: Deterministic, inspectable, matches the FR-G6 ladder and the +no-bundling decision (policy_rule_name is the broadest *rung*, not a bundle). + +**Alternatives**: LLM-proposed labels (deferred — must never enter the runtime +matcher; could later enrich labels only); fixed single generalization (rejected — +removes human control over breadth). + +## RG3. Telegram multi-step picker without busting callback_data + +**Decision**: Telegram `callback_data` is ≤ 64 bytes, too small for a predicate. +So: on **Always…** tap, the transport looks up the request's already-stored +`_Sent` entry, computes candidates via `grants.propose()`, stashes them in a +short-lived `_pending_picker[approval_id] = [candidates]`, and renders buttons +with `callback_data = pick:{approval_id}:{index}`. On the pick callback it reads +`_pending_picker[approval_id][index]`, creates the grant, resolves the approval +`allow`, and edits the message to confirm. A `pick:{approval_id}:cancel` returns +to the original Approve/Deny choice. + +**Rationale**: Keeps `callback_data` tiny and within Telegram limits; the +candidate set lives in process memory keyed by the approval, consistent with how +the transport already tracks `_Sent`. + +**Alternatives**: encode predicate in callback_data (rejected — 64-byte limit); +inline-query/web-app picker (rejected — overkill, needs more bot setup). + +## RG4. TTL expiry + max-grants enforcement + +**Decision**: Expiry is enforced **lazily at match time** (an expired grant never +matches — FR-G8) *and* swept periodically by a lightweight asyncio task started +in the FastAPI lifespan (default every 60 s) to reclaim memory and keep +`/rules` honest. `create()` enforces `max_grants` under the store lock; over the +cap it raises a typed error the transport surfaces in Telegram ("grant limit +reached"). On shutdown the store is simply dropped (no drain needed — grants are +advisory, losing them just re-prompts). + +**Rationale**: Lazy check guarantees correctness even if the sweeper lags; the +sweeper is for hygiene, not correctness. Cap mirrors the pending-approval cap +(FR-034) for symmetric backpressure. + +**Alternatives**: per-grant timer tasks (rejected — needless task churn); +sweep-only without lazy check (rejected — a lag window could match an expired +grant, violating FR-G8). + +## RG5. Scope derivation and matching + +**Decision**: A grant's scope is one of `session|workspace|project|instance| +global` with a captured `value`. Matching compares against the request fields: +`session`→`session_id`, `workspace`→`workspace`, `project`→`project`, +`instance`→`instance_id` (falling back to configured instance id), `global`→ +always. A grant matches only if the request's corresponding field **equals** the +grant's captured value (or scope is `global`). If the request lacks the field the +scope keys on, it is **no match** (fail-closed). + +**Rationale**: Exact-equality scope keeps matching deterministic and prevents a +grant created in project A from firing in project B. Fail-closed on missing +fields avoids accidental broadening. + +**Alternatives**: hierarchical/prefix scope (deferred — adds matching complexity +not needed in v1); ignore scope (rejected — removes the primary blast-radius +control). + +## RG6. Auto-approval audit + digest + +**Decision**: Each short-circuit auto-approval emits a structured INFO log +`auto_approved {approval_id, grant_id, kind, command|host|path-summary, +latency_ms}` (no secrets/bodies — FR-017) and increments the grant's +`uses_count`/`last_used_at`. A digest task (lifespan, default hourly; configurable +or off) sends the authorized chat a summary ("auto-approved N ops via M grants in +the last hour") when count > 0. Digest cadence reuses the same lightweight task +loop pattern as the sweeper. + +**Rationale**: Satisfies FR-G11 (no silent standing grants) without a per-event +Telegram message (which would defeat the purpose). Structural-only fields honor +the logging redaction already built. + +**Alternatives**: per-auto-approval Telegram message (rejected — noise, negates +the feature); no digest (rejected — violates the "never silent" intent). + +## RG7. Interaction with the existing intake flow + +**Decision**: The short-circuit is the **first** step of `POST /v1/approve`, +before the shutdown/health gate and before `reserve()`. Order: parse → (if grants +enabled and not paused) `match` → on hit return `allow` (+`grant_id`) and log; +on miss fall through to the existing shutdown/health → clamp → reserve → send → +await pipeline unchanged. Timeout/capacity/duplicate logic is untouched on the +miss path. + +**Rationale**: Auto-approved requests should cost nothing (no slot, no transport +call), and a paused/disabled grant system must transparently fall back to the +proven v1 behavior. + +**Alternatives**: match after reserve (rejected — wastes a slot and a send for an +auto-approvable request). + +## Resolved summary + +| Topic | Resolution | +|-------|-----------| +| Store ownership / access | app-owned, injected into transport; match in server (RG1) | +| Candidate templates | deterministic tightest-first per kind + policy_rule_name rung (RG2) | +| Telegram picker | stash candidates, `pick:{id}:{index}` callback_data (RG3) | +| TTL + cap | lazy-at-match + periodic sweep; cap on create (RG4) | +| Scope matching | exact-equality per scope field, fail-closed on missing (RG5) | +| Audit + digest | structural INFO log + periodic digest, no secrets (RG6) | +| Intake ordering | short-circuit first, miss falls through unchanged (RG7) | +| Agentsh promotion/signing | external OQ1 — deferred, not a v1 blocker | + +All v1 implementation unknowns resolved — ready for Phase 1 / tasks. diff --git a/specs/007-notifier-sidecar/research.md b/specs/007-notifier-sidecar/research.md new file mode 100644 index 0000000..acd0f74 --- /dev/null +++ b/specs/007-notifier-sidecar/research.md @@ -0,0 +1,124 @@ +# Phase 0 Research: Notifier Sidecar + +This document resolves the open technical decisions implied by the spec and the repo's existing conventions. Each entry is a Decision / Rationale / Alternatives triple. No `NEEDS CLARIFICATION` markers remained in the spec after `/speckit.clarify`; the items below are technical-approach choices, not requirement gaps. + +## R1. FastAPI + Telegram on one asyncio event loop + +**Decision**: Run the FastAPI app under uvicorn programmatically (`uvicorn.Server(Config(...)).serve()`); start the `python-telegram-bot` `Application` (initialize → start → `updater.start_polling()`) inside the FastAPI **lifespan** startup and stop it (stop polling → stop → shutdown) in lifespan shutdown. Both share the single running asyncio loop. + +**Rationale**: The spec mandates long-polling (no public URL) and a single shared loop. PTB v21's `Application` is asyncio-native and composes with an externally-owned loop when driven via its lower-level `initialize/start/updater.start_polling` methods rather than the blocking `run_polling()` (which calls `asyncio.run` and owns the loop — incompatible with uvicorn already owning it). The lifespan hook is the documented FastAPI seam for background services. + +**Alternatives considered**: +- `Application.run_polling()` directly — rejected: it creates/owns its own loop and blocks, conflicting with uvicorn. +- Separate thread for the bot with its own loop — rejected: cross-loop `asyncio.Event` resolution is error-prone; the spec explicitly wants one loop. +- Webhook mode — rejected by constraint (needs a public URL). + +## R2. Pending-approval registry and timeout race + +**Decision**: `PendingApprovals` holds `approval_id -> PendingApproval`, where each entry owns an `asyncio.Future[ApprovalDecision]`. Intake flow: validate → reject if `approval_id` already pending (409) → reject if at capacity (503) → deliver notification via transport → only then register the entry → `await asyncio.wait_for(future, timeout=clamped_timeout)`. On `TimeoutError`, resolve to fail-secure deny (reason `timeout`), call `transport.cancel`/edit, and return 408. A callback resolves the Future via `loop.call_soon_threadsafe`-safe path (same loop, so direct `set_result` guarded by "still pending"). All mutations go through an `asyncio.Lock` to keep cap-check + insert atomic. + +**Rationale**: Futures (not Events) carry the decision payload directly. Registering only *after* successful send (R5) keeps the cap meaningful and satisfies FR-010a. The lock makes the capacity gate (FR-034) and duplicate-id gate (FR-003a) race-free. "Still pending" guards make late/duplicate callbacks no-ops (edge cases). + +**Alternatives considered**: +- `asyncio.Event` + side dict for the decision — rejected: two structures to keep consistent; Future is one. +- Register before send — rejected: would hold a cap slot for a request that failed to notify (violates FR-010a intent). +- Per-entry lock only — rejected: capacity is a global invariant, needs a registry-level lock. + +## R3. Timeout clamping and defaults + +**Decision**: Effective timeout = `min(max(request.timeout_seconds or default, 1), max_timeout_seconds)`, with `default_timeout_seconds` and `max_timeout_seconds` from config (defaults 300 / 1800). Clamp silently (no error) per FR-006. + +**Rationale**: Matches FR-006 directly; a silent clamp is friendlier than rejecting and keeps the caller contract simple. A 1 s floor prevents zero/negative pathological waits. + +**Alternatives considered**: 400 on over-max — rejected: FR-006 says clamp, not reject. + +## R4. Fail-secure decision algebra + +**Decision**: A single internal resolver yields exactly one terminal outcome per approval: `ALLOW` only from an authorized human tapping Approve; everything else (`Deny` tap, timeout, transport/send failure, shutdown, lost connection) is `DENY` or no response at all. HTTP mapping: human decision → 200; timeout → 408 with `{decision: deny, reason: "timeout"}`; validation → 400; duplicate-id → 409; transport down / no config / shutting down / at capacity / send failure → 503. + +**Rationale**: Centralizes FR-008's invariant in one place that tests can target (SC-005). The 409 for duplicate-id (FR-003a) is the one non-spec-enumerated code; documented in the contract as a sub-case of "client error" from FR-003a/FR-001's 400 family — see R9. + +**Alternatives considered**: Returning 200-with-deny on transport failure — rejected: 503 lets the caller distinguish "no human saw this" from "human denied". + +## R5. Getting the image build context onto the host + +**Decision**: Force-include the minimal Docker build context into the wheel next to the already-included `ansible/` tree, at `remo_cli/notifier_build/` (containing `Dockerfile`, `pyproject.toml`, `uv.lock`, `README.md`, and the `remo_cli` source needed to `pip install ".[notifier]"`). The `remo_notifier` role copies this directory to `remo_notifier_source_dir` on the host and runs `community.docker.docker_image` with `source: build`. A resolver in the role (or passed as an extra-var by the CLI) locates the installed build context via `importlib`/package path, mirroring how `core/config.get_ansible_dir()` resolves the bundled `ansible/`. + +**Rationale**: A `pip install remo-cli` user has no repo checkout, yet the spec requires v1 to build on the host. Shipping the build context in the wheel (the same trick already used for `ansible/`) makes `remo notifier deploy` work from a bare install. The Dockerfile at repo root (`notifier/Dockerfile`, per spec) is the source of truth; the wheel-included copy is produced by a hatch force-include mapping. + +**Alternatives considered**: +- Build in CI, pull from ghcr.io — explicitly deferred by the spec (out of scope v1). +- Copy from the user's repo checkout — rejected: not present for pip installs. +- Build an sdist on the host — rejected: heavier and still needs the source shipped. + +**Layout contract (U1)**: the bundled context at `remo_cli/notifier_build/` MUST reproduce a repo-root-relative tree — `Dockerfile`, `pyproject.toml`, `README.md`, `uv.lock`, `src/remo_cli/…` — so the *same* `notifier/Dockerfile` builds identically from the repo root and from the on-host copy. The role (T025) copies this directory verbatim, preserving layout; the CLI (T028) resolves the installed path and passes it as an extra-var. `uv.lock` is shipped only for an optional `uv sync --frozen` locked build; a plain `uv pip install ".[notifier]"` does not consume it (I1). + +**Risk/Note**: Force-including `src/` doubles some files in the wheel. Keep the included context minimal (only what `.[notifier]` install needs). Confirm image stays <250 MB (AC-2) — Python 3.13-slim + these deps is ~180–220 MB. + +## R6. Secret handling and rotation + +**Decision**: The bot token is read once from `bot_token_file` (default `/run/secrets/telegram_bot_token`) at startup and kept only in memory. The TOML config never contains the token. Rotation: operator rewrites the secret file and restarts the service; additionally the process installs a `SIGHUP` handler that re-reads the secret file and re-initializes the bot (supports the spec's "rotation via kill -HUP" note) — best-effort, not a v1 acceptance criterion. structlog is configured with a processor that drops/reduces known-sensitive keys; the token is never passed to a logger. + +**Rationale**: Satisfies FR-019 (separate file, never config), FR-017 (no secrets in logs), and the out-of-scope note that HUP-rotation is supported without a CLI wrapper. + +**Alternatives considered**: Env-var token — rejected: env is visible in `/proc` and process listings; file with `0400` is tighter and rotatable. + +## R7. structlog configuration matching remo + +**Decision**: Configure structlog at process start: ISO timestamp, level, logger name, JSON renderer in the container (machine-readable for journald), key-value renderer if a TTY. INFO+ emits only structural fields (`approval_id`, `decision`, `latency_ms`, `transport`, `pending_count`). A dedicated DEBUG path may log operation/workspace/raw-body. A redaction processor strips `bot_token`, `authorization`, and full request bodies from any event not explicitly at DEBUG. + +**Rationale**: FR-017 + the "do not log secrets" constraint. JSON to stdout is the right shape for `docker logs` → journald → `remo notifier logs`. + +**Alternatives considered**: stdlib logging — rejected: spec names structlog and it gives cleaner structured redaction. + +## R8. CLI subcommand transport (SSH vs direct HTTP) + +**Decision**: `remo notifier {status,logs,test,restart}` operate **over SSH to the host** using the existing `core/ssh.build_ssh_opts` + a subprocess `ssh` invocation, consistent with `remo shell`/`cp`. `status` runs `curl -sf http://{bind}:{port}/v1/health` on the host; `test` runs `curl` POSTing a test `ApprovalRequest` to the same bridge address from the host; `logs` runs `journalctl -u remo-notifier.service [-f] [-n N]`; `restart` runs `sudo systemctl restart remo-notifier.service`. `deploy` runs the `notifier_deploy.yml` playbook via `core/ansible_runner.run_playbook` with `-i "{host},"` and `-e ansible_user=...`, exactly like `hetzner update`. Every subcommand resolves the host via `core/known_hosts` + `core/picker` (fuzzy pick when no host given, FR-031). + +**Rationale**: The listener is bound to the bridge and unreachable from the laptop (FR-021), so the laptop cannot curl it directly — it must hop through the host over SSH. Reusing the established SSH/ansible/picker helpers keeps UX identical to existing commands and adds zero laptop runtime deps. + +**Alternatives considered**: +- Direct HTTP from laptop — rejected: bridge binding makes it unreachable by design. +- A second control endpoint exposed publicly — rejected: widens attack surface against FR-021. + +## R9. HTTP status code for duplicate approval_id + +**Decision**: Return **409 Conflict** for a duplicate-of-pending `approval_id` (FR-003a), documented in the contract alongside the spec's 400/408/503. Body: `{ "error": "duplicate_approval_id", "approval_id": "..." }`. + +**Rationale**: 409 is the precise semantic ("conflict with current resource state — already pending"). The spec phrased FR-003a as "a client-error signal"; 409 is the most accurate client-error code and is additive to the spec's enumerated set, not contradictory. + +**Alternatives considered**: 400 — acceptable per the literal spec wording but less precise; 200-attach (idempotent) was rejected during clarification (Option B not chosen). + +## R10. Ansible collection + role wiring + +**Decision**: Add `community.docker (>=4.0.0)` to `ansible/requirements.yml` (used by `community.docker.docker_image`). The `remo_notifier` role declares `meta/main.yml` `dependencies: [{role: docker}]`. A new top-level `notifier_deploy.yml` playbook (`hosts: all`, `become: true`) applies the role; `remo notifier deploy` invokes it. Inclusion in the standard configure flow is via `configure_dev_tools.yml` guarded by `configure_remo_notifier | default(true) | bool` (FR-033), following the existing toggle pattern. + +**Rationale**: `docker_image`/`docker_container`/systemd are the idiomatic Ansible path here; `community.docker` is the only missing collection. The standalone `notifier_deploy.yml` lets `remo notifier deploy ` target an already-provisioned host without re-running the whole configure, while the toggle lets first-time provisioning include it. + +**Alternatives considered**: +- `docker_container` module instead of a systemd unit running `docker run` — rejected: the spec mandates a systemd unit with specific `docker run` hardening flags and `Restart=always`; a templated unit matches the spec exactly and keeps restart semantics in systemd. +- Reuse a provider configure playbook — rejected: would couple the notifier to one provider; a dedicated playbook is provider-agnostic. + +## R11. Test strategy and new dev dependencies + +**Decision**: Add `pytest-asyncio` and `httpx` to the `dev` extra. Tests: `test_state.py` drives the registry directly (register/resolve/timeout/cancel/cap/duplicate-id, including concurrency via `asyncio.gather`); `test_server.py` uses FastAPI `TestClient` (sync) for status-code coverage and `httpx.AsyncClient` + ASGI transport for the await-until-decision and timeout (408) paths with a fake transport that resolves on command; `test_telegram.py` injects a mocked `Bot` into the PTB `Application` and asserts message text, inline keyboard `callback_data`, the callback→resolve path, message edits, and `cancel`; `test_cli_notifier.py` patches the SSH/subprocess and ansible-runner seams and a fake known-hosts registry. Target >85% line coverage on `src/remo_cli/notifier/` (SC/AC-8). + +**Rationale**: Mirrors the spec's testing section and existing `tests/` patterns (`tests/unit`, `tests/integration`, `conftest.py`). A fake `NotificationTransport` makes server timeout/fail-secure tests deterministic without Telegram. + +**Alternatives considered**: Live Telegram in CI — rejected: non-deterministic, needs secrets; reserved for the manual `remo notifier test` acceptance step. + +## Resolved unknowns summary + +| Topic | Resolution | +|-------|-----------| +| One-loop FastAPI + PTB | lifespan start/stop, low-level PTB API (R1) | +| Registry + timeout race | Future + registry lock, register-after-send (R2) | +| Timeout clamp | min/max with config bounds (R3) | +| Fail-secure mapping | central resolver; 200/408/400/409/503 (R4, R9) | +| Build context to host | force-include in wheel, role copies + builds (R5) | +| Secrets | file-only token, in-memory, HUP re-read, redaction (R6, R7) | +| CLI transport | SSH hop reusing existing helpers (R8) | +| Ansible wiring | community.docker, role meta→docker, deploy playbook + toggle (R10) | +| Tests/deps | pytest-asyncio + httpx, fake transport, Bot mock (R11) | + +All items resolved — ready for Phase 1. diff --git a/specs/007-notifier-sidecar/spec.md b/specs/007-notifier-sidecar/spec.md new file mode 100644 index 0000000..287279e --- /dev/null +++ b/specs/007-notifier-sidecar/spec.md @@ -0,0 +1,212 @@ +# Feature Specification: Notifier Sidecar — Telegram approval bridge for agentsh + +**Feature Branch**: `007-notifier-sidecar` +**Created**: 2026-05-31 +**Status**: Draft +**Input**: User description: "Notifier Sidecar: Telegram approval bridge for agentsh — a long-running HTTP daemon in an OCI container on each remo-provisioned instance that receives agentsh approval webhooks, delivers them to a human via a Telegram bot, and returns the human's decision back to agentsh within the approval timeout." + +## Overview + +When the future agentsh execution-layer security model decides an operation needs human approval, it must reach a human who is not watching the terminal. The notifier is the push channel that closes that gap: it accepts an approval request from a devcontainer on the same instance, alerts a human on Telegram, and returns the human's allow/deny decision synchronously — failing secure (deny) if no one answers in time. + +This spec establishes the notifier service and, critically, the **durable wire protocol** that future approval emitters integrate against. Telegram is the only delivery channel implemented in v1, but the request/response contract and the operator workflow (deploy, test, observe) are the lasting deliverables. + +## Clarifications + +### Session 2026-05-31 + +- Q: When the notifier restarts while approvals are in flight, what happens to those pending approvals? → A: They are lost; the caller's open connection drops and the caller (agentsh) treats a dropped connection as a fail-secure deny. No persistence. +- Q: Who is authorized to answer an approval? → A: Exactly one pre-configured Telegram chat per instance (single human/chat). Multi-recipient and identity-aware routing are out of scope for v1. +- Q: What is the network exposure of the approval endpoint? → A: Bound to the host's container-bridge address only — reachable by co-located devcontainers but not from outside the host; no transport encryption or caller authentication in v1. +- Q: Is there a bound on how many approvals can be pending at once (any bridge container can POST with no caller auth)? → A: Yes — a configurable maximum concurrent pending approvals (default 50); beyond it, new requests are rejected with a service-unavailable signal until capacity frees up. +- Q: What happens when delivering the notification for one specific request fails while the transport is otherwise up? → A: Fail that request immediately with a service-unavailable signal and hold no pending slot; the caller retries or applies its own deny. +- Q: What happens when a request arrives carrying an approval_id that is already pending? → A: Reject the duplicate with a client-error signal while the original is still pending; the original keeps running. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Human approves or denies an operation from their phone (Priority: P1) + +A devcontainer's security layer encounters an operation its policy flags for human approval and sends an approval request to the notifier. The human receives a Telegram message describing the operation (project, command, rule, instance) with **Approve** and **Deny** buttons. They tap one; the decision is delivered back to the waiting caller, and the Telegram message updates to show who decided and when. + +**Why this priority**: This is the entire reason the service exists. Without it there is no human-in-the-loop approval channel and the agentsh security model cannot function. It is the MVP — everything else supports this loop. + +**Independent Test**: Send a single approval request to a running notifier; confirm a Telegram message with two buttons arrives, tap a button, and confirm the caller receives the matching decision within seconds and the Telegram message reflects the outcome. + +**Acceptance Scenarios**: + +1. **Given** a running, configured notifier, **When** an approval request arrives and the human taps **Approve** within the timeout, **Then** the caller receives an "allow" decision identifying the responder and the elapsed time, and the original Telegram message is edited to show it was approved. +2. **Given** a running, configured notifier, **When** an approval request arrives and the human taps **Deny**, **Then** the caller receives a "deny" decision and the Telegram message is edited to show it was denied. +3. **Given** an approval request with a 5-second timeout, **When** no human responds, **Then** the caller receives a fail-secure "deny" decision marked as a timeout within roughly 5–6 seconds, and the Telegram message is edited to show it timed out. + +--- + +### User Story 2 - Operator deploys the notifier to an instance (Priority: P1) + +An operator who already manages remo-provisioned hosts wants the approval channel running on a specific host. From their laptop they run a single deploy command naming the host. The notifier is built/installed and started on that host as a managed background service, bound so that only co-located devcontainers can reach it. The command fails loudly if the required Telegram credentials are not configured. + +**Why this priority**: An approval loop nobody can stand up has no value. Deployment must be a one-command operator action consistent with the rest of the remo CLI, and it is a prerequisite for Story 1 to run anywhere real. + +**Independent Test**: Run the deploy command against a fresh host that has only the base container runtime, then confirm the service is active and answering its health probe, with no manual steps in between. + +**Acceptance Scenarios**: + +1. **Given** a reachable host and valid Telegram credentials available to the operator, **When** the operator runs the deploy command for that host, **Then** the notifier service ends up active and running and the deploy reports success. +2. **Given** missing or empty Telegram credentials, **When** the operator runs the deploy command, **Then** it fails with a clear message naming what is missing and does not start a broken service. +3. **Given** an already-deployed host, **When** the operator runs deploy again with a rebuild flag, **Then** the service is rebuilt and restarted and ends up active and running. + +--- + +### User Story 3 - Operator verifies wiring end-to-end (Priority: P2) + +After deploying (or when first setting up Telegram), the operator runs a test command for the host. This sends a clearly test-labeled approval through the full path; the human receives a test Telegram message and taps a button; the command reports the round-trip succeeded and shows the decision. This confirms the bot token, chat ID, network binding, and decision return path all work without waiting for a real agentsh event. + +**Why this priority**: First-time Telegram setup has several independent failure points (wrong token, wrong chat ID, bot never messaged, port binding). A dedicated test command turns a frustrating multi-step debug into one observable action. Valuable but not required for the core loop to function. + +**Independent Test**: Run the test command against a deployed host; confirm a test-labeled Telegram message arrives, tap a button, and confirm the command prints the returned decision. + +**Acceptance Scenarios**: + +1. **Given** a deployed, healthy notifier, **When** the operator runs the test command, **Then** a test-labeled approval arrives on Telegram and the operator's tapped decision is reported back by the command. +2. **Given** a host where the notifier is not running, **When** the operator runs the test command, **Then** it reports the service is unreachable rather than hanging indefinitely. + +--- + +### User Story 4 - Operator observes and controls a running notifier (Priority: P3) + +An operator wants to check whether a host's notifier is healthy, read its logs to diagnose an issue, or restart it after a configuration change. The CLI exposes status, logs (optionally followed), and restart subcommands for a named host, matching the host-selection UX of the rest of remo (including fuzzy host picking when no host is named). + +**Why this priority**: Day-2 operability. The service can be deployed and exercised without these, but they make ongoing operation and debugging practical. Lowest priority because each is a thin convenience over what an operator could otherwise do by hand. + +**Independent Test**: Against a deployed host, run status and confirm it reports the health summary; run logs and confirm service log lines stream; run restart and confirm the service returns to active. + +**Acceptance Scenarios**: + +1. **Given** a deployed notifier, **When** the operator runs status, **Then** the current health summary (status, version, transport, uptime, count of pending approvals) is displayed. +2. **Given** a deployed notifier, **When** the operator runs logs with the follow option, **Then** service log output streams until the operator stops it. +3. **Given** a deployed notifier, **When** the operator runs restart, **Then** the service stops and comes back to active and running. +4. **Given** no host is named on any of these subcommands, **When** the operator runs it, **Then** they are offered an interactive fuzzy picker of known hosts, consistent with other remo commands. + +--- + +### Edge Cases + +- **Late or duplicate response**: A human taps a button for an approval that already timed out, or taps twice. The notifier ignores responses for approvals that are no longer pending and does not change an already-decided outcome. +- **Unauthorized responder**: A button tap or message arrives from a chat that is not the configured authorized chat. It is ignored and has no effect on any approval. +- **Malformed request**: An approval request that fails schema validation is rejected with a client error and never produces a Telegram message. +- **Transport unavailable**: The Telegram channel cannot be reached, or no human-side configuration is loaded. New approval requests are refused with a service-unavailable signal rather than silently hanging. +- **Shutdown with in-flight approvals**: The service is stopping while approvals await a human. In-flight callers are released (their connections drop / they receive a service-unavailable signal); no decision is fabricated as "allow". Restart loses all pending approvals (no persistence), and callers treat the dropped connection as deny. +- **Oversized timeout**: A request asks for a timeout beyond the configured maximum. The notifier clamps to the maximum rather than honoring an unbounded wait. +- **Secret rotation**: The Telegram bot token changes. The operator rotates it by updating the stored secret and restarting (or signaling) the service; the token is never read from the main config file or emitted in logs. +- **Concurrent approvals**: Multiple approval requests are pending at once. Each is tracked independently by its own identifier; a decision on one never resolves another. +- **Duplicate request identifier**: A request arrives bearing an approval identifier already in flight. It is rejected with a client error and the original pending approval is untouched; no second notification is sent. +- **Capacity reached**: The maximum number of concurrent pending approvals is already in flight. New requests are refused with a service-unavailable signal and no notification, until a pending approval resolves. +- **Notification send failure**: Delivery of a specific request's notification fails though the transport is otherwise up. That request fails immediately with a service-unavailable signal and never occupies a pending slot. + +## Requirements *(mandatory)* + +### Functional Requirements + +#### Approval intake and response + +- **FR-001**: The notifier MUST accept approval requests over HTTP at a versioned endpoint and validate them against the published request schema, rejecting malformed requests with a client-error status and no notification. +- **FR-002**: The notifier MUST hold the caller's request open and return a single response only when a decision exists — a human decision, a timeout, or service shutdown. +- **FR-003**: For each request, the notifier MUST generate an approval identifier if the caller did not supply one, and echo the approval identifier in the response. +- **FR-003a**: If a request supplies an approval identifier that matches an approval already pending, the notifier MUST reject the duplicate with a client-error signal and leave the original pending approval running undisturbed (one live approval per identifier). +- **FR-004**: On a human allow/deny, the notifier MUST return a success status with the decision, the responder's identity, an optional reason, the decision timestamp, and the elapsed latency. +- **FR-005**: On expiry of the request's timeout with no human response, the notifier MUST return a distinct timeout status carrying a fail-secure "deny" decision marked with a timeout reason. +- **FR-006**: The notifier MUST clamp any requested timeout to a configured maximum and apply a configured default when the request omits a timeout. +- **FR-007**: When the transport is unreachable, no human-side configuration is loaded, or the service is shutting down, the notifier MUST refuse new approval requests with a service-unavailable signal rather than hanging or fabricating an allow. +- **FR-034**: The notifier MUST enforce a configurable maximum number of concurrent pending approvals (default 50). While that limit is reached, the notifier MUST reject further approval requests with a service-unavailable signal — without sending a notification — until a pending approval resolves and frees capacity. This bounds memory use and protects the single human channel from notification flooding by a co-located container. + +#### Fail-secure guarantees + +- **FR-008**: The notifier MUST NEVER return "allow" except as the direct result of an authorized human explicitly approving. Every other terminal outcome (timeout, error, shutdown, lost connection) MUST resolve to deny or to no decision at all. +- **FR-009**: The notifier MUST NOT persist approval state; a restart loses all in-flight approvals, and callers whose connection drops MUST be able to treat that as a deny. + +#### Telegram delivery + +- **FR-010**: For each accepted request, the notifier MUST send a message to the single configured authorized chat describing the project, the operation (kind, command, arguments), the triggering rule, the human-readable policy message, the instance identifier, and the decision deadline, with inline **Approve** and **Deny** controls. +- **FR-010a**: If delivering the notification for a specific request fails while the transport is otherwise available, the notifier MUST fail that request immediately with a service-unavailable signal and MUST NOT hold a pending slot for it. A request is registered as pending only once its notification has been delivered. +- **FR-011**: The notifier MUST accept a decision only from the configured authorized chat and MUST ignore responses originating from any other chat. +- **FR-012**: The notifier MUST correlate each response to its originating approval by identifier and MUST ignore responses for approvals that are no longer pending (already decided, timed out, or unknown). +- **FR-013**: After an outcome (approved, denied, timed out, or cancelled), the notifier MUST update the original Telegram message to reflect that outcome, including the responder and time for human decisions. +- **FR-014**: The notifier MUST use a delivery mode that requires no publicly reachable inbound URL for the bot, so the service needs no public endpoint. +- **FR-015**: The notification channel MUST be pluggable behind a single transport abstraction so additional channels can be added later without changing the intake or state logic. Only the Telegram channel is implemented in v1, and the abstraction MUST also expose a cancellation path for an approval resolved by other means. + +#### Health and observability + +- **FR-016**: The notifier MUST expose an unauthenticated health endpoint returning status, service version, active transport name, uptime, and the count of currently pending approvals. +- **FR-017**: The notifier MUST NOT emit secrets or sensitive request content at normal log levels; the bot token, raw request bodies, and workspace paths MUST appear only at debug level, while normal levels carry structural metadata only (such as approval identifier, decision, and latency). + +#### Configuration and secrets + +- **FR-018**: The notifier MUST read its configuration from a single file, validate it strictly, and reject unknown configuration keys with a clear error. +- **FR-019**: The notifier MUST read the Telegram bot token from a separate secret file at startup (never from the main configuration file) so the secret can be rotated by editing that file and restarting or signaling the service. +- **FR-020**: Configuration MUST include the listening parameters, default and maximum approval timeouts, the maximum number of concurrent pending approvals, the selected transport, the transport's authorized chat identifier and secret location, and the instance identifier shown to humans. + +#### Network exposure + +- **FR-021**: The deployed notifier MUST be reachable by devcontainers co-located on the same host but MUST NOT be reachable from outside the host; it is bound to the host's container-bridge address. + +#### Operator deployment and lifecycle + +- **FR-022**: The remo CLI MUST provide a command to deploy the notifier to a named known host — including a fresh host that has only the base container runtime — ending with the service running as a managed, auto-restarting background unit. +- **FR-023**: Deployment MUST fail loudly with a clear message if the required Telegram credentials are not available, and MUST NOT leave a half-configured or broken service running. +- **FR-024**: Deployment MUST provide an option to force a rebuild of the service image on the host. +- **FR-025**: Deployment MUST verify the service answers its health endpoint before reporting success, and MUST fail if it does not come up within a reasonable bound. +- **FR-026**: The deployed service MUST run with hardened, least-privilege container settings (non-root user, dropped capabilities, read-only root filesystem) and MUST restart automatically if it exits. + +#### Operator verification and observability commands + +- **FR-027**: The remo CLI MUST provide a test command that sends a clearly test-labeled approval through the full path against a named host and reports the returned decision, surfacing an unreachable service rather than hanging. +- **FR-028**: The remo CLI MUST provide a status command that retrieves and displays the named host's notifier health summary. +- **FR-029**: The remo CLI MUST provide a logs command for a named host, with options to follow the stream and to limit the number of lines. +- **FR-030**: The remo CLI MUST provide a restart command that restarts the named host's notifier service. +- **FR-031**: Every notifier CLI subcommand that takes a host MUST offer an interactive fuzzy host picker when no host is named, consistent with existing remo commands. + +#### Non-regression and packaging + +- **FR-032**: Adding the notifier MUST NOT change the behavior of any existing remo command, and the laptop-side CLI install MUST NOT be forced to pull in the notifier's runtime dependencies (they live behind an optional install). +- **FR-033**: Deployment MUST be integrable into the existing per-host configuration flow via a toggle (`configure_remo_notifier`), consistent with how other optional host components are enabled/disabled. The toggle MUST default to **off** — the notifier requires Telegram credentials and its preflight fails loudly without them (FR-023), so a normal provision must not attempt to deploy it; the primary path is the explicit `remo notifier deploy ` command. (Corrected from an initial default-on, which broke credential-less provisioning.) + +### Key Entities *(include if feature involves data)* + +- **Approval Request**: An inbound ask for a human decision. Carries an optional approval identifier, an opaque session identifier, an operation descriptor (kind, command, arguments, optional path/remote host/port, nesting context and depth), the triggering rule name, a human-readable policy message, the workspace path, the instance identifier, the project name, a requested timeout, and a submission timestamp. +- **Approval Response**: The outbound decision. Carries the approval identifier, the decision (allow or deny), the responder identity, an optional reason, the decision timestamp, and the measured latency. +- **Pending Approval**: The in-memory record of a request awaiting resolution, keyed by approval identifier, holding the means to deliver the eventual decision back to the waiting caller. Exists only between intake and resolution; never persisted. +- **Transport**: A notification channel responsible for delivering an approval request to a human, collecting their decision, and reflecting cancellation. Telegram is the only concrete transport in v1. +- **Notifier Configuration**: The validated settings governing listening parameters, timeout bounds, the selected transport and its parameters (authorized chat, secret location, message formatting), and the instance identity shown to humans. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: After a human taps a decision button, the waiting caller receives the matching decision within 5 seconds. +- **SC-002**: A request whose timeout elapses with no human response returns a fail-secure deny within roughly its timeout window (within about 1 second of the deadline). +- **SC-003**: An operator can take a fresh host to a running, health-passing notifier with a single deploy command and no manual post-steps, and the service reports healthy within 5 seconds of starting. +- **SC-004**: A first-time user can go from zero (no bot) to a confirmed working approval round-trip using only the documented setup steps plus the deploy and test commands. +- **SC-005**: No terminal outcome other than an explicit authorized human approval ever yields "allow" — verified across timeout, malformed-request, transport-down, and shutdown scenarios. +- **SC-006**: Secrets and sensitive request content (bot token, raw bodies, workspace paths) never appear in logs at normal verbosity. +- **SC-007**: Installing and using the notifier introduces no change to any existing remo command's behavior, and a default laptop install does not acquire the notifier's extra runtime dependencies. +- **SC-008**: The published wire protocol (request schema, response schema, status codes, timeout and cancellation semantics) is documented completely enough that an independent emitter could integrate against it without reading the notifier's code. + +## Assumptions + +- The target hosts already have a container runtime available (provisioned by the existing remo host configuration), so the notifier role can depend on it rather than installing it. +- Exactly one human (one Telegram chat) is authorized per instance in v1; multi-recipient routing, first-response-wins, and identity-aware authorization are deferred. +- Telegram is the only delivery channel in v1; the transport abstraction keeps other channels possible but none are built. +- The threat model excludes cross-host eavesdropping in v1: the endpoint is bound to the host container bridge and is not encrypted or caller-authenticated. This is revisited only if multi-host networking becomes a feature. +- The service image is built on the host during deployment in v1; publishing a prebuilt image is deferred until the container surface stabilizes. +- Callers (agentsh) own their own timeout-action policy; the notifier's responsibility ends at returning a deny on timeout. + +## Out of Scope + +- Any non-Telegram delivery channel (Slack, Discord, ntfy, email). +- Multi-user / multi-chat fanout, first-response-wins routing, and identity-aware authorization. +- Persistent storage of in-flight approvals across restarts. +- A web UI or dashboard for in-flight or historical approvals. +- Approval history / audit export beyond what the host's service logs already provide. +- Integration with downstream consumers (e.g. workflow status announcers or lifecycle-event emitters); this spec ships the foundation and the wire-protocol contract only. +- Prebuilt published container images. +- Transport encryption (TLS) for the listener. +- A dedicated CLI wrapper for bot-token rotation (rotation is supported by editing the secret file and restarting/signaling). diff --git a/specs/007-notifier-sidecar/tasks-addendum-001.md b/specs/007-notifier-sidecar/tasks-addendum-001.md new file mode 100644 index 0000000..9282b51 --- /dev/null +++ b/specs/007-notifier-sidecar/tasks-addendum-001.md @@ -0,0 +1,174 @@ +# Tasks — Addendum 001: Standing Grants ("Always") + +**Input**: `addendum-001-standing-grants.md`, `plan-addendum-001.md`, +`research-addendum-001.md`, `data-model-addendum-001.md`, +`contracts/grant-schema.md`, `contracts/openapi.yaml` (updated), `quickstart.md`. + +**Tests**: INCLUDED — the parent spec requests tests (§9) and SC-G1…SC-G5 +require them. Test tasks precede their implementation within each story. + +**Scope**: Additive to the shipped v1 notifier. Task IDs are namespaced `TA###` +to avoid collision with the base `tasks.md` (T001–T046). All paths repo-relative. + +**Shared-file note**: `transports/telegram.py` is touched by US-G1, US-G2, and +US-G3; `server.py` by Foundational + US-G3; `tests/notifier/test_telegram.py` by +US-G1/US-G2/US-G3. Same-file tasks are sequenced, not parallel — flagged below. + +--- + +## Phase 1: Setup + +- [X] TA001 Add the `[grants]` config block to `GrantsConfig` planning surface: extend `src/remo_cli/notifier/config.py` with a `GrantsConfig` Pydantic model (`extra="forbid"`: `enabled` bool=true, `default_ttl_seconds` int≥1=28800, `max_grants` int≥1=100, `allow_global_scope` bool=true, `digest_interval_seconds` int≥0=3600) and add `grants: GrantsConfig = Field(default_factory=GrantsConfig)` to `NotifierConfig`. (data-model-addendum §GrantsConfig; FR-G12) +- [X] TA002 [P] Add the rendered `[grants]` section to `ansible/roles/remo_notifier/templates/notifier.toml.j2` (enabled/default_ttl_seconds/max_grants/allow_global_scope/digest_interval_seconds from role vars). +- [X] TA003 [P] Add grant defaults to `ansible/roles/remo_notifier/defaults/main.yml` (`remo_notifier_grants_enabled: true`, `_default_ttl_seconds: 28800`, `_max_grants: 100`, `_allow_global_scope: true`, `_digest_interval_seconds: 3600`). +- [X] TA003a [P] Add `GrantsConfig` validation cases to `tests/notifier/test_config.py`: a `[grants]` block parses with defaults; an unknown `[grants]` key is rejected (`extra="forbid"`); `default_ttl_seconds < 1` and `max_grants < 1` are rejected; `digest_interval_seconds = 0` is accepted (disables digest). (validates TA001; C1 / Constitution IV parity) + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: The grant domain model + store + matcher + proposer that every story +depends on. **No story work begins until this is complete.** + +- [X] TA004 Create `src/remo_cli/notifier/grants.py` models: enums (`GrantScopeType`, `ArgMatchType`, `HostMatchType`), `GrantPredicate`, `GrantScope`, `Grant`, `CandidateGrant` (Pydantic v2; reuse `OperationKind` from `models.py`). Include `predicate.matches(operation, policy_rule_name)`, `scope.matches(request)`, `grant.active(now)`, `grant.matches(request, now)` — all deterministic and fail-closed per data-model-addendum + grant-schema. (FR-G4/G5) +- [X] TA005 Implement `GrantStore` in `src/remo_cli/notifier/grants.py`: in-memory `dict[str,Grant]` + `asyncio.Lock` + `max_grants` + `paused`; methods `match(request, now)`, `create(grant)` (raises `GrantLimitReached` at cap), `list()`, `revoke(id)`, `set_paused()`, `sweep(now)`. (FR-G4/G7/G8/G9/G10; research RG1/RG4/RG5) +- [X] TA006 Implement `GrantStore.propose(request) -> list[CandidateGrant]` in `src/remo_cli/notifier/grants.py` (pure): deterministic tightest-first templates per kind + `policy_rule_name` broadest rung, each paired with the narrowest covering scope plus one widened rung; **return at most 4 candidates**. (FR-G6; research RG2) +- [X] TA007 [P] Add optional `grant_id: str | None = None` to `ApprovalResponse` in `src/remo_cli/notifier/models.py`. (data-model-addendum; backward-compatible) +- [X] TA008 [P] Unit tests `tests/notifier/test_grants.py`: predicate match per kind (command exact/prefix/glob, file path-glob+op, network exact/suffix+port, signal, policy_rule_name rung); scope equality + missing-field fail-closed; `active()`/expiry; `match()` returns None when paused/expired/revoked/mismatch; `create()` cap → `GrantLimitReached`; `revoke()`; `sweep()`; `propose()` ordering tightest-first; concurrent `create()` respects cap via `asyncio.gather`; **a freshly constructed `GrantStore` is empty (SC-G5 restart fail-closed: a new process holds no grants → re-prompts).** (validates TA004–TA006) + +**Checkpoint**: grant model, store, matcher, proposer importable and unit-tested. + +--- + +## Phase 3: User Story G1 — Human grants "Always" for a class (Priority: P1) 🎯 MVP + +**Goal**: Tapping **Always…** approves this request and creates a standing grant; a subsequent matching request is auto-approved server-side with no Telegram. + +**Independent Test**: Tap Always on a request; send a second matching request → `200 allow` in <500 ms, `responder: rule:{id}`, `grant_id` set, no Telegram, audit line logged. + +### Tests for US-G1 (write first) + +- [X] TA009 [P] [US-G1] Server short-circuit tests in `tests/notifier/test_server.py`: with a grant pre-seeded in `app.state.grant_store`, a matching `POST /v1/approve` returns 200 `allow` with `responder` `rule:{id}` + `grant_id`, **no** transport send and **no** pending slot reserved; a non-matching request falls through to the existing flow; with grants disabled or paused the short-circuit is skipped. (validates TA011; FR-G1/G2/G3, research RG7) +- [X] TA010 [P] [US-G1] Telegram "Always" flow tests in `tests/notifier/test_telegram.py` (Bot mock): approval keyboard now includes `always:{id}`; tapping it renders a picker with `pick:{id}:{index}` candidates from `propose()`; selecting one calls `grant_store.create(...)` and resolves the approval `allow` (+grant_id); `pick:{id}:cancel` restores Approve/Deny; unauthorized chat / unknown id ignored; at `max_grants` the approval still allows once but reports the limit. (validates TA012/TA013; grant-schema "Telegram surface") + +### Implementation for US-G1 + +- [X] TA011 [US-G1] Wire the intake short-circuit into `src/remo_cli/notifier/server.py`: construct `GrantStore` in `create_app` (expose as `app.state.grant_store`), inject it into the transport; at the top of `POST /v1/approve` (before shutdown/health gate and `reserve`), if grants enabled and not paused, `match()` → on hit return 200 `allow` (+`grant_id`, responder `rule:{id}`), increment `uses_count`, emit the structural audit log; on miss fall through unchanged. Start the TTL `sweep` loop in the lifespan. (FR-G1/G2/G3/G11; research RG4/RG6/RG7) +- [X] TA012 [US-G1] Add the third button + picker to `src/remo_cli/notifier/transports/telegram.py`: render `[✅ Approve] [⏩ Always…] [❌ Deny]`; on `always:{id}` compute `grant_store.propose(request)`, stash candidates keyed by approval id, render `pick:{id}:{index}` + Cancel buttons (labels include scope); keep `callback_data` ≤64 bytes. (research RG3; grant-schema) +- [X] TA013 [US-G1] Handle the pick callback in `transports/telegram.py`: on `pick:{id}:{index}` create the grant (TTL applied via config), resolve the approval `allow` with `grant_id`, edit the message to confirm (`⏩ Always: