docs(plan): add broker audit and recommendations

plan/docs/security_audit.md

@@ -0,0 +1,150 @@
+# PostgreSQL Broker Security and Readiness Audit
+
+**Audit date:** 2026-09-14  
+**Scope:** Repository review of the PostgreSQL broker source, schema, build/release automation, documentation, and available test suite.
+
+## Executive summary
+
+The project is an early, functional prototype and is not production-ready. The working tree was clean at review time. Development activity is limited to five commits from 2–3 January 2026, with no release tag.
+
+Implemented capabilities include a Go CLI, multi-database configuration, embedded schema installation, PostgreSQL LISTEN/NOTIFY, queue/worker processing, instance heartbeats, CI workflows, and a single end-to-end happy-path test.
+
+The highest-priority work is establishing a safe execution security model, fixing single-instance ownership, making installation idempotent, and adding recovery-focused test coverage.
+
+## Findings
+
+### Critical: arbitrary SQL privilege escalation
+
+Any role allowed to call `broker_add_job` can enqueue SQL that the broker later executes under the broker database role. PostgreSQL functions are executable by `PUBLIC` by default unless privileges are explicitly revoked. This can allow a lower-privileged caller to cause arbitrary SQL to run with the broker account's rights.
+
+Relevant files:
+
+- `pkg/broker/install/sql/procedures/05_broker_add_job.sql`
+- `pkg/broker/install/sql/procedures/02_broker_run.sql`
+
+Required remediation:
+
+1. Define the broker security model and the trusted roles allowed to enqueue work.
+2. Revoke default `PUBLIC` access to broker functions and grant only the minimum required permissions.
+3. Use a least-privileged broker role.
+4. Consider a constrained job model rather than accepting arbitrary SQL text.
+
+### Critical: the broker is not PostgreSQL Row-Level Security aware
+
+The broker has no tenant or execution-principal field on jobs, no RLS policy design, and does not apply `run_as` when executing a job. Queued SQL consequently executes in the broker session's effective role. If that role owns application tables, has `BYPASSRLS`, or is otherwise exempt from policies, it can bypass tenant isolation. Conversely, simply enabling RLS can cause workers to see no eligible rows or fail unexpectedly because the worker has no policy context.
+
+Relevant files:
+
+- `pkg/broker/install/sql/tables/03_broker_jobs.sql`
+- `pkg/broker/install/sql/procedures/02_broker_run.sql`
+- `pkg/broker/install/sql/procedures/03_broker_set.sql`
+
+Required remediation:
+
+1. Define a tenant and execution-principal model before enabling RLS. Add an immutable tenant/principal reference to jobs and schedules; do not infer it from a mutable job name or SQL string.
+2. Separate roles: a non-login schema/migration owner, a least-privileged broker runtime role that does not own tenant tables and does not have `BYPASSRLS`, and narrowly scoped application/enqueuer roles.
+3. Enable and force RLS on tenant-owned application tables, with policies based on a transaction-local, validated tenant context or a trusted execution role. Account explicitly for PostgreSQL table-owner and `BYPASSRLS` exemptions.
+4. Do not allow arbitrary queued SQL to choose its own identity through `SET ROLE`, `SET SESSION AUTHORIZATION`, or unvalidated session settings. Prefer approved job types/procedures with explicit grants; if impersonation is required, map an approved tenant identity to a controlled execution context in the same transaction.
+5. Decide whether broker metadata is global operational data or tenant data. If tenant-scoped, apply policies to `broker_jobs` and `broker_schedule` too, while preserving a deliberate worker claim path. If global, revoke direct tenant access rather than relying on RLS.
+6. Add integration tests using at least two tenant roles that prove cross-tenant reads, writes, and queued execution are denied, including scheduled work and retry/recovery paths.
+
+### High: single-instance protection is unsafe
+
+On registration response `retval = 3` (another broker is active), the application retrieves the active instance ID and continues to start workers. This contradicts the documented one-instance-per-database guarantee and can result in multiple broker processes operating under one instance record.
+
+The SQL registration check is also race-prone: it counts active rows without a uniqueness constraint or advisory lock, so concurrent registrations can both pass the check.
+
+Relevant files:
+
+- `pkg/broker/database_instance.go`
+- `pkg/broker/install/sql/procedures/04_broker_register_instance.sql`
+
+Required remediation:
+
+1. Treat an existing active instance as a startup failure; do not reuse its ID.
+2. Enforce ownership atomically with a database constraint and/or transaction-scoped advisory lock.
+3. Define stale-instance takeover rules using heartbeat expiry and an ownership token.
+
+### High: dependency semantics are incorrect
+
+Dependencies are treated as satisfied whenever they are not pending. A dependency that is running, failed, or cancelled therefore permits execution. Dependencies also use non-unique job names, making the dependency relationship ambiguous.
+
+Relevant file: `pkg/broker/install/sql/procedures/01_broker_get.sql`
+
+Required remediation:
+
+1. Model dependencies by job IDs in a join table.
+2. Permit execution only when every dependency has completed successfully.
+3. Define the terminal behavior for failed or cancelled dependencies.
+
+### High: installer is not rerunnable
+
+Tables use `IF NOT EXISTS`, but trigger creation does not. Re-running `install` against an existing schema fails when creating existing triggers. Installation has no schema migration/version tracking and is not atomic across the whole schema.
+
+Relevant files:
+
+- `pkg/broker/install/sql/tables/02_broker_schedule.sql`
+- `pkg/broker/install/sql/tables/03_broker_jobs.sql`
+- `pkg/broker/install/install.go`
+
+Required remediation:
+
+1. Make trigger creation idempotent or explicitly replace/recreate triggers safely.
+2. Introduce a schema-version table and ordered migrations.
+3. Make each migration transactional where PostgreSQL permits it.
+4. Add a test that runs installation twice and verifies the result.
+
+### Medium: advertised features are incomplete
+
+Schedules exist only as a table; no scheduler implementation creates jobs from cron expressions. `run_as`, worker idle timeout, and notification retry settings are configured or stored but unused. Queue creation also hardcodes one worker per queue.
+
+Relevant files:
+
+- `pkg/broker/database_instance.go`
+- `pkg/broker/config/config.go`
+- `pkg/broker/install/sql/tables/02_broker_schedule.sql`
+
+Required remediation: either implement these capabilities and test them, or remove them from the public documentation and configuration until supported.
+
+### Medium: startup failure leaks resources and active state
+
+If registration succeeds but queue or listener startup fails, the database instance has not yet been appended to the broker's managed instance list. Cleanup therefore does not stop its partially started resources, close its connection, or mark its database registration as shut down.
+
+Relevant file: `pkg/broker/broker.go`
+
+Required remediation: make startup transactional: on every failure after connection, stop started queues, close the adapter, and release or mark the instance registration before returning the error.
+
+### Medium: configuration permits broken runtime modes
+
+Configuration validation does not reject zero or negative queue, fetch, and timer values. These can leave jobs unprocessed or cause a zero-duration timer loop. TLS defaults to `disable`.
+
+Relevant file: `pkg/broker/config/config.go`
+
+Required remediation: validate all runtime limits as positive, set safe production TLS defaults, and require an explicit opt-out for plaintext local connections.
+
+### Medium: release and documentation drift
+
+The release workflow injects `main.version`, while the program exposes `main.Version`; release binaries will keep reporting `dev`. The Makefile build time is a fixed literal timestamp. README examples include an obsolete constructor signature, refer to a missing `examples/` directory, reference a nonexistent `make test` target, and show an outdated `broker_add_job` parameter list.
+
+Relevant files:
+
+- `.github/workflows/release.yml`
+- `Makefile`
+- `README.md`
+
+Required remediation: align release linker flags with exported variable names, generate build time dynamically, and make documentation executable/validated in CI.
+
+## Validation performed
+
+- `go vet ./...` passed.
+- `go build ./cmd/broker` passed.
+- Race-enabled tests for `./pkg/...` passed, but these packages contain no tests.
+- The integration suite could not reach its PostgreSQL test database at `localhost:5433` from the audit sandbox because local socket access is blocked. Runtime integration behavior is therefore unverified in this audit environment.
+
+## Recommended next milestone
+
+1. Lock down execution permissions and the job security model.
+2. Implement atomic exclusive-instance ownership and stale-owner recovery.
+3. Add idempotent, versioned schema migrations.
+4. Add unit and integration coverage for authorization, duplicate starts, crash recovery, dependency outcomes, notification loss, and repeat installation.
+5. Reconcile implemented behavior, configuration, release metadata, and documentation.