Skip to content

docs(agami): least-privilege read-only DB role — SELECT-only deploy obligation (ACE-036)#110

Open
sandeep-agami wants to merge 5 commits into
mainfrom
ACE-036-least-privilege-readonly-role
Open

docs(agami): least-privilege read-only DB role — SELECT-only deploy obligation (ACE-036)#110
sandeep-agami wants to merge 5 commits into
mainfrom
ACE-036-least-privilege-readonly-role

Conversation

@sandeep-agami

Copy link
Copy Markdown
Collaborator

Spec: ACE-036

Summary

Elevate agami-core's read-only datasource role from recommended to a stated deploy obligation, and make the app-role vs operator/owner-role split explicit. Docs-only — no code or behaviour change. Implements ACE-036 (High lane) under feature F9 — safe SQL execution, where the read-only role is the PRIMARY, non-bypassable control (a login that can only SELECT can't write/DDL/COPY/reach files/network even if the app-layer guard were bypassed).

Changes

  • plugins/agami/shared/readonly-grants.md (primary): two-posture framing (single-player = recommended; hosted deploy via DATASOURCE_URL = required); the app-role vs operator/owner-role split (writes stay on a separate privileged identity; never point DATASOURCE_URL at owner creds); the datasource-vs-store distinction (agami's own APP_DATABASE_URL/AGAMI_DB_URL store stays read-write); a "what the role does and does NOT guarantee" note (integrity/confinement yes; runaway/recon bounding is app-side); explicit "SELECT-only is the whole guarantee here" for BigQuery-IAM and SQLite/DuckDB; an optional PUBLIC-revoke hardening line for PG ≤14 / Redshift.
  • deploy/README.md: security note elevated from "safest thing to connect" to the required DATASOURCE_URL posture, with the datasource/store precision.
  • docs/self-hosting.md: add DATASOURCE_URL to the config table (read-only role required; a separate DB from the store; accurate env-DSN scheme list) and link the recipe.
  • plugins/agami/shared/credentials-format.md: note single-player recommended vs hosted required, same recipe + split.
  • CHANGELOG.md: [Unreleased] entry. No version bump (docs-only — per maintainer decision).

Acceptance (ACE-036 success criteria)

  • SC1 — SELECT-only obligation stated for a hosted deploy (app read-only; separate write role), not optional/recommended.
  • SC2 — every engine has a complete grant block or an explicit "SELECT-only is the whole guarantee" note (SQLite/DuckDB, BigQuery-IAM).
  • SC3 — self-host deploy docs link the recipe as the required DATASOURCE_URL posture.
  • SC4 — role guarantees integrity/confinement; availability/recon are app-side (no reader assumes it bounds runaways).
  • Per contract Q1 — no mandated role-level timeout / search_path / catalog-revoke (the PG hardening is optional).

Checklist

  • ruff check clean; 44 targeted deploy/env/marketplace tests pass; docs-only (no .py, no version change).
  • No secrets (placeholders only).
  • Agami review panel run — security sign-off (High lane): 0 must-fix, 2 nits, both fixed in the last commit.
  • Human PR approval (High-lane security sign-off) — requested.

Out of scope (per spec)

Role-level hardening beyond SELECT-only · app-layer enforcement code (sql_guard/runtime/execute_sql) · provisioning the role in a customer DB · the DB-role-denies-write E2E test (→ ACE-040).

🤖 Generated with Claude Code

sandeep-agami and others added 4 commits July 11, 2026 17:13
Reframe readonly-grants.md from "optional / recommended" to the two-posture
model: single-player = strongly recommended; hosted deploy = a requirement.
Make the app-role vs operator/owner-role split explicit (writes stay on a
separate privileged identity; never point DATASOURCE_URL at owner creds), and
call out that agami's own store (APP_DATABASE_URL) is a separate read-write DB.

Add the integrity/confinement-vs-availability residual note: the role is the
primary non-bypassable control for what a query can touch, but does NOT bound
runaways or block recon — timeout + row cap + recon denial are app-side. Make
"SELECT-only is the whole guarantee here" explicit for BigQuery-IAM and
SQLite/DuckDB.

Spec: ACE-036

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…L posture

The self-host guide (F4) now references readonly-grants.md as the required
datasource posture, not a recommendation:
- deploy/README.md: elevate the security-note from "safest thing to connect"
  to "required posture for a deploy"; spell out the datasource role vs the
  APP_DATABASE_URL store (which stays read-write).
- docs/self-hosting.md: add DATASOURCE_URL to the config table (must be a
  read-only SELECT-only role; separate DB from the store) + link the recipe.
- credentials-format.md: note single-player = recommended, hosted deploy =
  required, same recipe, and the app-role vs operator-role split.

Spec: ACE-036

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Unreleased entry only; no version bump (docs-only, accumulates until release).

Spec: ACE-036

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- readonly-grants.md: add an OPTIONAL hardening note for PostgreSQL ≤14 /
  Redshift — a fresh role inherits CREATE-on-public + TEMP via the built-in
  PUBLIC role, so the flagship grant isn't strictly SELECT-only there until you
  REVOKE those from PUBLIC (noted as database-wide; PG15+ already drops the
  public CREATE default). Optional per the SELECT-only baseline (Q1), so it
  makes the "physically unable to run DDL" claim precise without mandating it.
- docs/self-hosting.md: the DATASOURCE_URL env-DSN channel only parses
  postgres/redshift/mysql/snowflake/bigquery/sqlite — replace the vague "etc."
  with the real scheme list and point other engines at the file credentials
  channel, so an operator on SQL Server/Oracle/Databricks/Trino doesn't hit an
  "Unsupported scheme" wall.

Spec: ACE-036

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 12, 2026 00:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates Agami’s documentation to make a least-privilege, SELECT-only datasource identity an explicit deploy obligation (for DATASOURCE_URL), clarifies the datasource vs metadata-store split, and adds engine-specific guidance for how “read-only” is guaranteed across supported backends (ACE-036 / F9 safe SQL execution).

Changes:

  • Elevates read-only datasource identity from “recommended” to required for hosted/self-host deployments, with a clear app-role vs operator/owner-role separation.
  • Clarifies that Agami’s store (AGAMI_DB_URL / APP_DATABASE_URL) remains read-write and is distinct from the datasource (DATASOURCE_URL).
  • Adds/expands notes on what the read-only role guarantees (integrity/confinement) vs what remains app-side (runaway bounding), plus optional PG/Redshift hardening and BigQuery/SQLite/DuckDB specifics.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
plugins/agami/shared/readonly-grants.md Reframes read-only datasource as primary deploy boundary; expands guarantees/non-guarantees and per-engine notes.
plugins/agami/shared/credentials-format.md Adds guidance distinguishing single-player vs deploy posture and links to the grants recipe.
docs/self-hosting.md Adds DATASOURCE_URL to the env var table and links to the read-only grants recipe.
deploy/README.md Elevates DATASOURCE_URL read-only role posture to required and clarifies datasource vs store.
CHANGELOG.md Adds an Unreleased entry describing the docs-only policy change (ACE-036).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread plugins/agami/shared/readonly-grants.md Outdated

## What the role does and does not guarantee

The read-only role is the **primary, non-bypassable** guarantee of **integrity and confinement**: SELECT-only means **no write / DDL / `COPY` / file access / server-side network call / cross-database reach** — and that holds even if the app-layer guard were bypassed. It does **not** bound a **runaway** query (a recursive CTE or cartesian join), and it does **not** stop schema/metadata **recon**: a per-statement **timeout + result-row cap**, and **recon-function denial + error-text sanitizing**, are enforced **app-side at the executor**, not by the role. So don't read "read-only" as "time-bounded" — the role confines *what* a query can touch, the app layer bounds *how much* it can consume.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch — fixed in 6a0f3e8. Confirmed against execute_sql.py: the executor surfaces raw DB error text today (_err(f"Postgres execution error: {e}", code=5)), and recon-function denial + error-text sanitizing are ACE-039 (a not-yet-shipped gap spec). Reframed the note to state only what's enforced now (the result-row cap) and to describe timeout + recon/error hardening as the app layer's responsibility rather than a shipped guarantee — so the doc no longer overstates protections while still keeping availability/recon off the role (ACE-036 SC4).

Comment thread docs/self-hosting.md Outdated
|----------|----------|---------|
| `AGAMI_DB_URL` | yes (server) | The store: `postgresql://…` in prod, `sqlite://…` for a small/local run. `APP_DATABASE_URL` is accepted as an alias for the cloud-platform convention (`AGAMI_DB_URL` wins if both are set). Unset ⇒ the local file path. |
| `AGAMI_DB_URL` | yes (server) | The **store** (agami's own metadata DB — the server migrates + writes it on boot): `postgresql://…` in prod, `sqlite://…` for a small/local run. `APP_DATABASE_URL` is accepted as an alias for the cloud-platform convention (`AGAMI_DB_URL` wins if both are set). Unset ⇒ the local file path. This is **not** your datasource — keep its user **read-write**. |
| `DATASOURCE_URL` | yes (to query) | The **datasource** agami runs generated SQL against, as a DSN. The env-DSN channel accepts `postgres` / `redshift` / `mysql` / `snowflake` / `bigquery` / `sqlite` schemes; other engines (SQL Server, Oracle, Databricks, Trino) connect via the file `credentials` channel instead. **Must point at a read-only (SELECT-only) role** — that role is the primary, non-bypassable safety boundary; see [read-only grants](../plugins/agami/shared/readonly-grants.md) for the per-engine recipe and the app-role vs operator-role split. A **different** database from the store above; never point it at owner/admin creds. Several datasources ⇒ one `DATASOURCE_URL__<TOKEN>` per datasource (token = the datasource id upper-cased, non-alphanumerics → `_`). |

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 6a0f3e8. Corrected the env-DSN scheme list to include the accepted aliases (postgresql, mariadb, bq) per _POSTGRES_SCHEMES/_MYSQL_SCHEMES/_BIGQUERY_SCHEMES, listed DuckDB among the schemeless engines that use the file credentials channel, and softened the Required cell to "to query (or file creds)" so it no longer implies DATASOURCE_URL is unconditionally required when the file-credentials channel is the path for schemeless engines.

- readonly-grants.md: the executor surfaces raw DB error text today
  (execute_sql.py `_err(f"... {e}")`); recon-function denial + error-text
  sanitizing are ACE-039 (not yet shipped). Reframe the residual note to state
  only what's enforced now (result-row cap) and describe timeout + recon/error
  hardening as the app layer's responsibility, not a shipped guarantee — still
  satisfies SC4 (availability/recon are app-side, not the role's).
- self-hosting.md: correct the env-DSN scheme list to include the accepted
  aliases (postgresql, mariadb, bq) and soften the "required" cell — schemeless
  engines (SQL Server, Oracle, Databricks, Trino, DuckDB) use the file
  credentials channel, so DATASOURCE_URL isn't unconditionally required.

Spec: ACE-036

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants