Skip to content

Add queryable_columns shape allow-list#4531

Merged
robacourt merged 7 commits into
mainfrom
rob/queryable-columns
Jun 8, 2026
Merged

Add queryable_columns shape allow-list#4531
robacourt merged 7 commits into
mainfrom
rob/queryable-columns

Conversation

@robacourt

@robacourt robacourt commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds queryable_columns as a server-side shape allow-list for columns that may be referenced by shape queries. This decouples the security boundary from the columns sync projection, so proxies can strictly control which columns are queryable while still allowing clients to request a narrower synced projection.

References security advisory: https://github.com/electric-sql/electric/security/advisories/GHSA-c82q-v86f-c87f

Changes

  • Parse and validate the new queryable_columns shape parameter.
  • Restrict top-level where, subset filters/orderings, and columns projections to queryable columns.
  • Default synced columns to queryable_columns when no columns projection is provided.
  • Add router and shape tests for normal and subset behavior.
  • Update HTTP docs, auth/shapes guides, OpenAPI, and add a changeset.

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.44%. Comparing base (7892079) to head (ad319b9).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4531       +/-   ##
===========================================
+ Coverage   32.48%   56.44%   +23.96%     
===========================================
  Files         216      358      +142     
  Lines       18368    39081    +20713     
  Branches     6478    10975     +4497     
===========================================
+ Hits         5967    22061    +16094     
- Misses      12369    16948     +4579     
- Partials       32       72       +40     
Flag Coverage Δ
packages/agents 70.75% <ø> (?)
packages/agents-mcp 77.54% <ø> (?)
packages/agents-mobile 66.92% <ø> (ø)
packages/agents-runtime 79.98% <ø> (?)
packages/agents-server 73.95% <ø> (+0.04%) ⬆️
packages/agents-server-ui 6.21% <ø> (ø)
packages/electric-ax 46.42% <ø> (?)
packages/experimental 87.73% <ø> (?)
packages/react-hooks 86.48% <ø> (?)
packages/start 82.83% <ø> (?)
packages/typescript-client 91.71% <ø> (?)
packages/y-electric 56.05% <ø> (?)
typescript 56.44% <ø> (+23.96%) ⬆️
unit-tests 56.44% <ø> (+23.96%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@netlify

netlify Bot commented Jun 8, 2026

Copy link
Copy Markdown

Deploy Preview for electric-next ready!

Name Link
🔨 Latest commit 53624ab
🔍 Latest deploy log https://app.netlify.com/projects/electric-next/deploys/6a26ccae07c57d00086451a8
😎 Deploy Preview https://deploy-preview-4531--electric-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@robacourt robacourt force-pushed the rob/queryable-columns branch from bc926d6 to c7cc9c3 Compare June 8, 2026 12:40
Comment thread website/docs/sync/api/http.md Outdated
Comment thread website/docs/sync/guides/auth.md Outdated
Comment thread website/docs/sync/guides/auth.md Outdated
Comment thread website/docs/sync/guides/auth.md Outdated
Comment thread website/docs/sync/guides/shapes.md Outdated
@robacourt robacourt marked this pull request as draft June 8, 2026 12:55
Comment thread website/docs/sync/guides/auth.md Outdated
Comment thread website/docs/sync/guides/auth.md Outdated
@robacourt robacourt marked this pull request as ready for review June 8, 2026 14:07

@alco alco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

Comment thread packages/sync-service/test/electric/plug/router_test.exs Outdated
Comment thread packages/sync-service/test/electric/plug/router_test.exs Outdated
Comment thread packages/sync-service/test/electric/plug/router_test.exs Outdated
@robacourt robacourt merged commit fcf9749 into main Jun 8, 2026
118 of 127 checks passed
@robacourt robacourt deleted the rob/queryable-columns branch June 8, 2026 15:00
@robacourt robacourt self-assigned this Jun 8, 2026
@alco

alco commented Jun 8, 2026

Copy link
Copy Markdown
Member

Took a focused pass looking for missed corner-cases in the new allow-list boundary. Two issues worth addressing, plus a docs nit.

1. Same-table subqueries bypass queryable_columns (when allow_subqueries is enabled)

The top-level WHERE refs are correctly filtered to the allow-list:

refs = column_info |> filter_columns(queryable_columns) |> Inspector.columns_to_expr()

…but subqueries are handled separately by build_shape_dependencies/2, which drops the allow-list:

shared_opts = Map.drop(opts, [:where, :columns, :queryable_columns, :relation])
#                                                  ^^^^^^^^^^^^^^^^^ dropped

Each sublink is rebuilt as its own dependency shape via new() with queryable_columns: nil, i.e. no restriction, and there's no guard that the subquery's FROM table differs from the root table. So a self-referential sublink re-opens the exact boolean-oracle the advisory describes:

where = id IN (SELECT id FROM users WHERE secret_token LIKE 'super%')

The outer id resolves against the filtered refs (fine), but the inner secret_token predicate is validated against the root table's full column set. Row inclusion in the outer result then leaks whether secret_token matches — even though it's neither queryable nor synced — which allows character-by-character enumeration again.

This is gated behind the allow_subqueries feature flag (not the default), so it's not a default-config hole — but since this PR is the security boundary, I think the subquery path deserves an explicit decision rather than silently inheriting the unrestricted column set.

Fix options:

  • Propagate queryable_columns into shared_opts for dependency shapes whose relation equals the root table (a sibling table's allow-list is effectively "all columns", so passing it through only constrains same-table subqueries); or
  • Reject sublinks targeting the root table when queryable_columns is set; or
  • At minimum, document the limitation and add a test pinning the chosen behaviour.

2. queryable_columns: [] empty-check is dead code

In validate_queryable_columns/3 the cond tests missing_pk_cols first:

missing_pk_cols = pk_cols -- queryable_columns   # for [] this is the full PK list
...
queryable_columns == [] -> "The list of queryable columns must not be empty"

For any table with a primary key, [] trips the missing_pk_cols branch first, so the user gets "must include all primary key columns" and the dedicated empty-list message is never reachable. Not a security problem — just a misleading error and an unreachable branch. Either move the empty check first or drop it.

Docs nit

Worth an explicit line that a proxy's own server-side where must only reference columns it also lists in queryable_columns, otherwise its legitimate auth filter gets rejected. The shapes.md / auth.md examples already do the right thing (org_id is in the list), but calling it out would save a footgun.

Suggested test for issue 1

General shape — a router test mirroring the existing queryable_users cases, but with the subquery feature flag enabled and a sublink on the same table referencing a non-queryable column:

@tag with_sql: [
       "CREATE TABLE queryable_users (id BIGINT PRIMARY KEY, name TEXT NOT NULL, secret_token TEXT NOT NULL)",
       "INSERT INTO queryable_users VALUES (1, 'Alice', 'supersecret123')",
       "INSERT INTO queryable_users VALUES (2, 'Bob', 'public')"
     ]
test "rejects subqueries referencing non-queryable columns on the root table", %{opts: opts} do
  conn =
    conn("GET", "/v1/shape", %{
      table: "queryable_users",
      offset: "-1",
      queryable_columns: "id,name",
      columns: "id,name",
      where: "id IN (SELECT id FROM queryable_users WHERE secret_token LIKE 'super%')"
    })
    # enable the allow_subqueries feature flag for this request
    |> Router.call(opts)

  assert %{status: 400} = conn
  assert %{"errors" => %{"where" => [message]}} = Jason.decode!(conn.resp_body)
  assert message =~ "unknown reference secret_token"
end

As written today this would return 200 and leak the oracle rather than 400, so it doubles as the failing repro.

robacourt added a commit that referenced this pull request Jun 9, 2026
…by the client (#4534)

PR #4531 made `queryable_columns` apply to normal shape `where` clauses,
but as
#4531 (comment)
pointed out, sensitive columns could still be accessed via subqueries.
Making arbitrary `where` clauses with subqueries secure would require
more than a root-table column allow-list: we would also need a table
allow-list plus per-table `queryable_columns` for every table referenced
by a subquery.

However, normal where clauses are supposed to be restricted and only set
server-side. There's no need to restrict them at all. The are to
restrict is the subset snapshots, where clients can change subset
`where` and `order_by`.

This PR makes the behavior consistent by allowing any server-defined
main `where` clause while keeping `queryable_columns` enforcement for
subset `where` and subset `order_by`. It also updates the docs to make
this distinction clear too.

## Summary
- allow main shape `where` clauses to reference any table column even
when `queryable_columns` is set
- keep `queryable_columns` enforcement for synced projections, subset
`where`, and subset `order_by`
- update docs, OpenAPI, and the existing changeset to describe the
narrower boundary
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.

2 participants