Remove column_access action field, fix 14 hook tests, and strengthen test/doc standards

- Remove `action` field from `ColumnAccessDef`; intent now derived solely from policy effect (permit=allowlist, deny=denylist)
- Fix 14 unit tests in `hooks/policy.rs` that incorrectly placed column_access deny obligations inside permit policies
- Add `tc_rf_01_neq_operator_quoted_column` integration test covering `!=` operator and double-quoted column identifiers
- Strip all stale `"action": "allow"/"deny"` fields from integration tests in `policy_enforcement.rs` and `protocol.rs`
- Fix `scripts/demo_ecommerce/policies.yaml`: change hide-credit-card and hide-product-financials to deny-effect policies
- Update `docs/permission-system.md` and `docs/roadmap.md` to remove action field references throughout
- Add security vector #20 to `docs/permission-security-tests.md`
- Update `proxy/CLAUDE.md`: document integration test infrastructure, TDD bug fix protocol, and comprehensive test coverage requirement

Author: jumpbetweenrows

docs/permission-security-tests.md

@@ -238,3 +238,22 @@ WITH data AS (SELECT * FROM orders) SELECT * FROM data
 **Defense**: `matches_pattern()` in `policy_match.rs` supports prefix glob: if the pattern ends with `*`, it uses `starts_with(prefix)` matching. `matches_schema_table()` delegates to `matches_pattern()` for both schema and table fields. The same function is used by both `PolicyHook` (query-time) and `compute_user_visibility()` (connect-time), ensuring consistent semantics.
 
 **Test**: 14 unit tests in `proxy/src/policy_match.rs` cover exact match, `*` wildcard, prefix glob on table, prefix glob on schema, combined globs, alias resolution, and non-matching cases (`orders_raw` does not match `raw_*`).
+
+---
+
+### 20. `column_access` obligation `action` field caused incorrect visibility in `policy_required` mode
+
+**Vector**: Admin creates a `permit`-effect policy intended to grant access to all columns (`columns: ["*"]`) but the stored obligation JSON has `"action": "deny"` (from a prior schema where the action was set per-obligation). In `policy_required` mode the table is completely inaccessible — the user sees a "table not found" error instead of data.
+
+**Bug**: The `column_access` obligation definition previously contained an `action` field (`"allow"` or `"deny"`). `compute_user_visibility()` and `ObligationEffects::collect()` both checked `col_def.action == "allow"` to decide whether a permit policy's `column_access` obligation grants table visibility. When `action` was `"deny"` inside a permit policy, the obligation was silently treated as a denylist and never added to `visible_tables`. In `policy_required` mode, `visible_tables` remained empty → the table was filtered out of the user's `SessionContext` → DataFusion returned "table not found".
+
+**Defense**: The `action` field was removed from `column_access` obligations entirely. Intent is now determined solely by the enclosing policy's `effect`:
+- `permit` policy + `column_access` → always an allowlist (adds to `visible_tables` and specifies visible columns)
+- `deny` policy + `column_access` → always a denylist (strips columns from results, does not grant access)
+
+The `ColumnAccessDef` struct no longer has an `action` field. The API validation layer (`dto.rs`) no longer requires or accepts `action` in `column_access` definitions. Existing stored obligations with an `action` field are silently ignored (serde unknown-field tolerance).
+
+**Test**:
+- Unit: `engine::tests::test_permit_column_access_wildcard_grants_full_visibility_policy_required` — permit policy with `column_access ["*"]` in a `policy_required` aliased datasource → table is visible, `visible_tables` non-empty.
+- Unit: `hooks::policy::tests::test_column_access_deny_no_table_permit` — deny policy with `column_access` in `policy_required` mode → `lit(false)` (deny policy alone does not grant table access).
+- Unit: `admin::policy_handlers::tests::create_policy_column_access_missing_columns_422` — `column_access` definition without `columns` field → `422`.

docs/permission-system.md

@@ -8,16 +8,16 @@ Permissions are defined as **policies**. A policy is a named, reusable unit that
 
 When a user runs a query:
 1. The proxy loads all **enabled** policies assigned to the datasource for that user.
-2. `deny` policies are evaluated first. A `row_filter` match on a deny policy rejects the query with an error. `column_access deny` obligations on deny policies are collected alongside those from permit policies (step 3).
-3. `permit` policies are applied — their obligations rewrite the query in-flight (row filters, column masks, column access controls). Column denies from both permit and deny policies are combined.
+2. `deny` policies are evaluated first. A `row_filter` match on a deny policy rejects the query with an error. `column_access` obligations on deny policies strip specific columns from results.
+3. `permit` policies are applied — their obligations rewrite the query in-flight (row filters, column masks, column access controls). Column denies from deny policies are applied on top.
 4. The rewritten query executes against the upstream database.
 
 ## Policy effects
 
-- **permit** — applies obligations to the query (row filtering, masking, etc.)
-- **deny** — primarily used to block access with an error. Can also carry `column_access deny` obligations to strip specific columns from results.
+- **permit** — applies obligations to the query (row filtering, masking, column allowlisting, etc.)
+- **deny** — primarily used to block access with an error. Can also carry `column_access` obligations to strip specific columns from results.
 
-Deny policies are evaluated before permit policies. If a deny policy has a matching `row_filter` obligation, the query is rejected immediately with an error. `column_access deny` obligations on deny policies strip columns from results just like they do on permit policies — they do not cause an error.
+Deny policies are evaluated before permit policies. If a deny policy has a matching `row_filter` obligation, the query is rejected immediately with an error. `column_access` obligations on deny policies strip columns from results — they do not cause an error.
 
 > **`is_enabled` flag**: only enabled policies are enforced. Disabling (or enabling) a policy removes (or adds) all its effects immediately — both for query-time enforcement and for schema visibility — without requiring a reconnect.
 >
@@ -57,23 +57,27 @@ Replaces a column's value with a masked expression in query results.
 
 When multiple mask policies target the same column, the one with the **lowest priority number** (highest precedence) wins.
 
-### column_access (deny)
+### column_access
 
-Removes the specified columns from query results entirely.
+Controls which columns a user can see. The **policy effect** determines the obligation's intent:
+
+- **In a `permit` policy** → acts as an **allowlist**: only the listed columns are visible; all others are hidden from schema metadata and query results. This is also the only obligation that makes a table accessible in `policy_required` mode.
+- **In a `deny` policy** → acts as a **denylist**: the listed columns are removed from schema metadata and query results.
 
 ```json
 {
   "schema": "public",
   "table": "customers",
-  "columns": ["ssn", "credit_card"],
-  "action": "deny"
+  "columns": ["ssn", "credit_card"]
 }
 ```
 
-Denied columns are unioned across all matching **enabled** policies, regardless of effect — if any enabled policy (permit or deny) denies a column, it is removed from the result. The column is also hidden from schema metadata (`information_schema.columns`) on the user's connection.
+Denied columns from `deny` policies are unioned — if any enabled deny policy removes a column, it is absent from results regardless of permit policies.
 
 If the query selects **only** denied columns (e.g. `SELECT ssn FROM customers`), the proxy returns SQLSTATE `42501` (insufficient privilege) with a message identifying the restricted columns rather than returning empty rows.
 
+Glob patterns are supported in the `columns` field — see [Column glob patterns](#column-glob-patterns-columns-field) below.
+
 ### object_access (deny)
 
 Hides entire schemas or individual tables from a user's virtual catalog — they become invisible in `information_schema.schemata`/`information_schema.tables`, SQL client sidebars, and query execution.
@@ -103,7 +107,7 @@ Unlike `column_access deny` (which strips columns from results), `object_access
 
 > **`column_mask` on deny policies is invalid.** The API returns `422 Unprocessable Entity` if you attempt to create or update a `deny`-effect policy with a `column_mask` obligation. The UI hides `column_mask` from the available obligation types when `deny` is selected. Only `column_access` and `object_access` obligations are supported on deny policies.
 
-> **Obligation definition validation.** The API validates obligation definitions at create/update time and returns `422 Unprocessable Entity` for malformed payloads: unknown `obligation_type` values, missing required fields (`schema`, `table`, `filter_expression`, `column`, `mask_expression`, `columns`, `action`), wrong field types, or invalid `action` values (`column_access` accepts `"allow"` or `"deny"`; `object_access` accepts `"deny"` only). Extra fields in the definition are permitted for forward compatibility.
+> **Obligation definition validation.** The API validates obligation definitions at create/update time and returns `422 Unprocessable Entity` for malformed payloads: unknown `obligation_type` values, missing required fields (`schema`, `table`, `filter_expression`, `column`, `mask_expression`, `columns`, `action`), wrong field types, or invalid `action` values (`object_access` requires `"deny"`). The `column_access` obligation has no `action` field — intent is determined by the enclosing policy's `effect`. Extra fields in the definition are permitted for forward compatibility.
 
 ## Template variables
 
@@ -173,47 +177,46 @@ Column visibility follows a **zero-trust** model in `policy_required` mode: a ta
 
 ### Obligation responsibility matrix
 
-| Obligation | Grants table access? | Grants column access? | Modifies data? |
-|---|---|---|---|
-| `row_filter` | **No** | No | Yes (filters rows) |
-| `column_mask` | **No** | No | Yes (transforms value) |
-| `column_access "allow"` | **Yes** | Yes (named columns only) | No |
-| `column_access "deny"` | **No** — does not unblock a table | Removes named columns | No |
-| `object_access "deny"` | Removes table/schema | Removes all | No |
+| Obligation | Policy effect | Grants table access? | Grants column access? | Modifies data? |
+|---|---|---|---|---|
+| `row_filter` | permit | **No** | No | Yes (filters rows) |
+| `column_mask` | permit | **No** | No | Yes (transforms value) |
+| `column_access` | **permit** | **Yes** | Yes (named columns only) | No |
+| `column_access` | **deny** | **No** — does not unblock a table | Removes named columns | No |
+| `object_access` | deny | Removes table/schema | Removes all | No |
 
-### `column_access "allow"` — the access grant obligation
+### `column_access` in a permit policy — the access grant obligation
 
-`column_access "allow"` is the **only** obligation that makes a table visible in `policy_required` mode. It also specifies which columns the user can see:
+`column_access` in a **permit** policy is the **only** obligation that makes a table visible in `policy_required` mode. It also specifies which columns the user can see:
 
 ```json
 {
   "schema": "public",
   "table": "customers",
-  "columns": ["id", "name", "email"],
-  "action": "allow"
+  "columns": ["id", "name", "email"]
 }
 ```
 
-With only this obligation, the user sees the `customers` table with exactly three columns. Any column not in the `columns` list is invisible in both schema metadata and query results.
+With only this obligation (on a permit policy), the user sees the `customers` table with exactly three columns. Any column not in the `columns` list is invisible in both schema metadata and query results.
 
 ### Composing access with row filters
 
-`column_access "allow"` and `row_filter` in the same policy stack correctly — the allow obligation grants access and column visibility, while the row filter restricts which rows are returned:
+`column_access` (permit) and `row_filter` in the same policy stack correctly — the `column_access` obligation grants access and column visibility, while the row filter restricts which rows are returned:
 
 ```json
 [
-  { "type": "column_access", "action": "allow", "schema": "public", "table": "customers", "columns": ["id", "name"] },
+  { "type": "column_access", "schema": "public", "table": "customers", "columns": ["id", "name"] },
   { "type": "row_filter",    "schema": "public", "table": "customers", "filter_expression": "organization_id = {user.tenant}" }
 ]
 ```
 
 Result: only `id` and `name` columns, filtered to the user's tenant rows.
 
-### `column_access "deny"` does not grant access
+### `column_access` in a deny policy does not grant access
 
-In `policy_required` mode, a policy that **only** has `column_access "deny"` obligations does **not** unblock the table. The table remains blocked by `lit(false)`. Use `column_access "allow"` first to grant access, then optionally layer `column_access "deny"` to remove specific columns.
+In `policy_required` mode, a **deny** policy with `column_access` obligations does **not** unblock the table. The table remains blocked by `lit(false)`. Use `column_access` on a **permit** policy first to grant access, then layer a `column_access` deny policy to strip specific columns.
 
-In `open` mode, `column_access "deny"` removes the specified columns from results (same behaviour as before).
+In `open` mode, `column_access` on a deny policy removes the specified columns from results.
 
 ### JOIN column scoping
 
@@ -443,16 +446,15 @@ Partially mask SSNs for support staff:
 ```
 
 ### Column access control (DS-10)
-Remove sensitive columns entirely:
+Remove sensitive columns entirely using a deny policy:
 ```yaml
 - name: hide-pii
-  effect: permit
+  effect: deny
   obligations:
     - type: column_access
       schema: public
       table: customers
       columns: [ssn, credit_card]
-      action: deny
 ```
 
 ### Admin full access (MT-05)

docs/roadmap.md

@@ -216,11 +216,11 @@ Each obligation type gets an optional `condition` field:
   "definition": { "schema": "hr", "table": "employees", "column": "ssn", "mask_expression": "'***-**-****'" }
 }
 
-// Column access deny - hide columns conditionally
+// Column access deny - hide columns conditionally (policy effect = "deny")
 {
   "obligation_type": "column_access",
   "condition": "user.clearance_level < 5",
-  "definition": { "schema": "secret", "table": "files", "action": "deny", "columns": ["content"] }
+  "definition": { "schema": "secret", "table": "files", "columns": ["content"] }
 }
 
 // Object access deny - hide tables conditionally  

proxy/CLAUDE.md

@@ -75,9 +75,40 @@ Policy CRUD handlers also call `state.proxy_handler.rebuild_contexts_for_datasou
 
 **Audit logging**: after each query, `PolicyHook` spawns a `tokio::spawn` task to insert a `query_audit_log` row asynchronously. The row captures `original_query`, `rewritten_query`, `policies_applied` (JSON with name+version snapshot), `client_ip`, and `client_info` (application_name from pgwire startup params).
 
+## Testing
+
+Data security and robustness are core product requirements. Every feature must ship with comprehensive unit and integration tests covering happy paths, edge cases, and security boundaries. Aim for best-in-class coverage — not just "it works", but "it cannot be bypassed".
+
+### Unit tests (`src/**`)
+Inline `#[cfg(test)]` modules in each source file. No external dependencies — run with `cargo test --lib`.
+
+### Integration tests (`tests/`)
+Two test binaries: `policy_enforcement.rs` (security/policy scenarios) and `protocol.rs` (pgwire protocol). Shared infrastructure in `tests/support/mod.rs`.
+
+Run with `cargo test --test policy_enforcement` or `cargo test --test protocol`. Require Docker — skipped gracefully via `require_postgres!()` if unavailable.
+
+**`ProxyTestServer::start()`** spins up a complete isolated stack per test:
+- One shared `testcontainers` Postgres container per test binary (`OnceLock`) — not per-test.
+- Fresh in-memory SQLite admin DB per test with all migrations applied.
+- Real `ProxyHandler` on a random TCP port with a live pgwire accept loop.
+- `axum_test::TestServer` wrapping the admin API for HTTP calls.
+
+**Conventions:**
+- Each test uses a unique upstream schema name (e.g. `"t1_rowfilt"`, `"tc_rf01"`) to avoid collisions during parallel execution.
+- TC-prefixed tests map to security vector numbers in `docs/permission-security-tests.md`.
+- Template vars in `filter_expression` must not be quoted: `tenant = {user.tenant}` ✓, `tenant = '{user.tenant}'` ✗.
+
+### Documentation requirements (non-optional)
+After completing any feature or adding tests, always update:
+- **`docs/permission-security-tests.md`** — add a new vector entry for any new attack surface or bypass that was tested (Vector → Bug/Defense → Test format).
+- **`docs/permission-system.md`** — keep the conceptual model, obligation descriptions, and examples in sync with the current implementation.
+
 ## Bug Fix Protocol
-- Every bug fix MUST include a regression test (unit or integration) that fails before the fix and passes after.
-- Security-related bugs (policy bypass, access control, injection) MUST also be documented in `docs/permission-security-tests.md` following the existing Vector → Bug → Defense → Test format.
+Use TDD: write the failing test(s) first to reproduce the bug, then fix the code until they pass. Never fix first and test after.
+
+1. Write unit and integration tests that reproduce the bug and fail on the current code. Cover all relevant edge cases — add as many tests as needed, not just one of each.
+2. Fix the code until the test passes.
+3. Security-related bugs (policy bypass, access control, injection) MUST also be documented in `docs/permission-security-tests.md` following the existing Vector → Bug → Defense → Test format.
 
 ## Known Issues
 - **regclass / regproc not supported** — `datafusion-table-providers` drops these columns. Catalog stores `arrow_type = NULL`; `build_arrow_schema` skips them.

proxy/src/admin/dto.rs

@@ -358,6 +358,8 @@ pub fn validate_obligation(obl: &ObligationRequest) -> Result<(), String> {
             require_str_field(def, "column_mask", "mask_expression")?;
         }
         ObligationType::ColumnAccess => {
+            // Intent (allowlist vs denylist) is determined by the enclosing policy's effect.
+            // No obligation-level 'action' field is required or accepted.
             require_str_field(def, "column_access", "schema")?;
             require_str_field(def, "column_access", "table")?;
             match def.get("columns") {
@@ -374,14 +376,6 @@ pub fn validate_obligation(obl: &ObligationRequest) -> Result<(), String> {
                     );
                 }
             }
-            let action = def.get("action").and_then(|v| v.as_str()).ok_or_else(|| {
-                "column_access obligation: missing required field 'action'".to_string()
-            })?;
-            if action != "allow" && action != "deny" {
-                return Err(
-                    "column_access obligation: 'action' must be 'allow' or 'deny'".to_string(),
-                );
-            }
         }
         ObligationType::ObjectAccess => {
             require_str_field(def, "object_access", "schema")?;