Skip to content

feat(jans-cedarling): Cedarling PostgreSQL Extension#13856

Open
haileyesus2433 wants to merge 94 commits into
mainfrom
jans-cedarling-12071
Open

feat(jans-cedarling): Cedarling PostgreSQL Extension#13856
haileyesus2433 wants to merge 94 commits into
mainfrom
jans-cedarling-12071

Conversation

@haileyesus2433

@haileyesus2433 haileyesus2433 commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Prepare


Description

Target issue

closes #12071

Implementation Details


Test and Document the changes

  • Static code analysis has been run locally and issues have been fixed
  • Relevant unit and integration tests have been added/updated
  • Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with docs: to indicate documentation changes or if the below checklist is not selected.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

Summary by CodeRabbit

  • New Features

    • PostgreSQL extension: row-level authorization, predicate pushdown, JWT/multi‑issuer & unsigned auth helpers, masking (rules/preview/apply), policy register/use/rollback, schema validation, entity mapping, observability (traces/status/explain), GUC runtime controls, installer, Docker image, and CLI for generating Cedar schemas.
  • Documentation

    • Full PostgreSQL integration guide with install options, examples, function reference, security and operational notes.
  • CI

    • New Postgres extension test job and optional packaging/provenance workflow; workspace test/bench runs now exclude the extension by default.

Review Change Stack

…go.toml`.

- Created `cedarling_pg` package with necessary configurations and dependencies.
- Added `.gitignore` for `cedarling_pg` to exclude build artifacts.
- Implemented a basic PostgreSQL extension with a sample function `hello_cedarling_pg`.
- Included regression test setup files for the new extension.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
@haileyesus2433 haileyesus2433 self-assigned this Apr 17, 2026
@coderabbitai

coderabbitai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new cedarling_pg PostgreSQL extension crate (engine, authz, mask, policy, observability), codegen CLI, installer/Docker packaging, generated SQL/catalog bundles, CI jobs for pgrx tests and per-PG packaging, workspace/manifest updates, comprehensive docs, and extensive unit/pg_test coverage.

Changes

Cedarling PostgreSQL Extension

Layer / File(s) Summary
All changes
jans-cedarling/cedarling_pg/*, jans-cedarling/cedarling_pg_codegen/*, .github/workflows/*, docs/cedarling/integrations/postgres.md, jans-cedarling/Cargo.toml, deny.toml, Dockerfiles, scripts`
Complete patch: new cedarling_pg extension crate and manifest, embedded SQL/catalog bundles, engine and lifecycle, authz bridge/cache/WHERE lowering, SQL-visible functions (authorized*, authorized_row, build_resource, tokens, mask, policy management, observability), masking engine and types, resource mapping, GUC registration, test fixtures/support, codegen CLI, installer and Docker packaging, CI workflow additions/changes (pgrx tests, packaging), documentation, and many unit/pg_test cases.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jans-cedarling-12071

@mo-auto

mo-auto commented Apr 17, 2026

Copy link
Copy Markdown
Member

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@mo-auto mo-auto added comp-jans-cedarling Touching folder /jans-cedarling kind-feature Issue or PR is a new feature request labels Apr 17, 2026
haileyesus2433 and others added 22 commits April 21, 2026 03:11
- Introduced `guc_config.rs` to define Grand Unified Configuration (GUC) parameters for Cedarling, including modes, fail modes, log levels, cache TTL, and token management.
- Implemented validation logic for the `cedarling.tokens` GUC in a new `validate.rs` file.
- Updated `lib.rs` to register GUCs during PostgreSQL extension initialization and added tests for default values and token validation.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
…ng `EntityData`, including error handling for invalid input.

- Introduced `token_sql.rs` with SQL-callable functions for managing `cedarling.tokens`, including setting, clearing, and retrieving tokens.
- Updated `lib.rs` to include new modules and added tests for token management functionality.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
…ization

- Added `authz_bridge.rs` for handling multi-issuer and unsigned authorization requests, including error handling.
- Introduced `engine.rs` to manage the global Cedarling engine initialization from a bootstrap configuration file.
- Updated `guc_config.rs` to include a new GUC for specifying the bootstrap configuration path.
- Created `token_bundle.rs` for parsing session token bundle JSON into Cedarling `TokenInput` values.
- Modified `lib.rs` to integrate new modules and ensure proper function exposure for testing.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
- Introduced `cedarling_authorized` and `cedarling_authorize_unsigned` functions for handling authorization requests with JWT and unsigned principals.
- Implemented internal logic for decision-making based on resource and action inputs, including error handling based on fail modes.
- Updated `guc_config.rs` to clarify behavior of the `cedarling.mode` GUC.
- Added tests for new authorization functions to ensure correct fail-open and fail-closed behavior.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
- Added `authz_cache.rs` to manage a process-local cache for successful Cedarling authorization decisions, utilizing the `sparkv` crate.
- Enhanced `cedarling_authorized_inner` and `cedarling_authorize_unsigned_inner` functions to leverage caching for improved performance.
- Updated `guc_config.rs` documentation to clarify caching behavior and configuration.
- Modified `Cargo.toml` to include new dependencies for caching functionality.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
- Introduced `extension_log.rs` to manage structured logging for Cedarling authorization processes, ensuring sensitive information is not logged.
- Updated `cedarling_authorized_inner` and `cedarling_authorize_unsigned_inner` functions to include detailed diagnostic logging for various error conditions.
- Enhanced documentation in `guc_config.rs` to clarify the logging behavior and its integration with authorization functions.
- Modified `lib.rs` to include the new logging module.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
- Introduced `catalog.rs` to define the schema and tables for Cedarling's masking and policy history.
- Added `error.rs` to implement a unified error handling mechanism for the extension, including detailed error types and logging capabilities.
- Enhanced `guc_config.rs` to include new GUCs for strategy and policy versioning, improving configuration flexibility.
- Updated `extension_log.rs` to support structured logging of errors and audit entries.
- Modified `lib.rs` to integrate new modules and ensure proper function exposure for testing.
- Added regression tests to verify catalog creation and error handling behavior.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
…gnostic functions

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
… retrieval function

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
…r RLS policies

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
…king and new test cases

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
- Added support for a new `cedarling.context` GUC to pass contextual information during authorization.
- Implemented validation for the context JSON object to ensure it is either empty or a valid JSON object.
- Updated authorization functions to utilize the new context, improving flexibility in policy evaluations.
- Introduced new tests to validate context handling and ensure robustness of the authorization logic.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
Signed-off-by: Haileyesus Ayanaw <85413826+haileyesus2433@users.noreply.github.com>
…dded a new `policy_versions` table to track policy versions.

- Introduced new GUC settings for `diff_mode` and `schema_validate_strict` to enhance configuration flexibility.
- Added row-level authorization helpers and resource building functions to improve authorization capabilities.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
…gnostics

- Introduced `AuthorizeOutcome` struct to encapsulate decision results along with request ID, policy hits, and diagnostic errors.
- Updated authorization functions to return `AuthorizeOutcome` instead of simple boolean values, improving error handling and traceability.
- Added `peek_cedarling` function to retrieve the current engine state without initialization.
- Enhanced observability by recording additional metadata in authorization traces, including resource type and ID.
- Updated status reporting to include new metrics for error tracking and policy updates.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
…le hash salt

- Added `MASK_HASH_SALT` GUC setting to configure salt for SHA-256 hash masking.
- Implemented `mask_hash_salt_bytes` function to retrieve the current salt as raw bytes.
- Created a new module for masking configuration, including default mask strategies for sensitive fields.
- Enhanced `cedarling_mask_row` and `cedarling_test_masking` functions to utilize the new masking strategies and salt.
- Introduced `MaskType` enum to define various masking strategies and their application logic.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
…logic

- Added `condition_sql` and `data_type` columns to the `mask_rules` table for improved masking configurations.
- Updated authorization functions to incorporate a masking strategy that stashes masked rows when access is denied.
- Enhanced `AuthorizationTrace` to include a `masked` field indicating when a deny decision results in a masked-row allow.
- Introduced new tests to validate the behavior of the masking strategy and ensure proper integration with existing authorization logic.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
- Refactored masking functionality to include new `MaskRule` struct for better handling of explicit masking rules.
- Updated `lookup_explicit_rules` to return `MaskRule` instances, allowing for conditional SQL evaluation in masking.
- Improved `cedarling_mask_row` and related functions to utilize the new masking structure and logic.
- Enhanced authorization functions to streamline decision-making and trace logging, including cache hit and error handling.
- Added tests to validate the new masking behavior and ensure robustness in authorization processes.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
- Introduced a new `install.sh` script to automate the build and installation of the cedarling_pg extension in a local pgrx-managed PostgreSQL environment.
- The script includes a health check that verifies the installation by checking for the presence of the extension, catalog schema, and required functions.
- Added support for environment variable overrides to customize PostgreSQL version, database name, and psql binary path.
- Provided usage instructions and options for release builds and skipping health checks.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
…ing_pg

- Introduced a new SQL file defining the schema for the cedarling_pg extension, including tables for masking rules, policy history, entity mapping, and policy versions.
- Added several authorization-related functions to enhance the extension's capabilities, including `cedarling_authorize_unsigned`, `cedarling_authorized`, and `cedarling_build_resource`.
- Included comments for clarity on the purpose of each table and function, improving documentation within the SQL file.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>

@tareknaser tareknaser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing my previous review comments. I have one follow up

if let Err(e2) = engine::rollback_policy() {
extension_log::log_diagnostic(
guc_config::CedarlingLogLevelGuc::Error,
&format!(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Follow-up to my earlier comment on set_policy_version_guc (which has been mostly addressed).

I noticed one edge case the new rollback path doesn't catch: engine::rollback_policy() returns Ok(None) when there is no previous engine to revert to (this is what happens on the very first cedarling_use_policy call in a backend)

The Ok(None) arm is silently ignored so I think it would work like:

  1. engine::use_policy succeeds and installs the new policy.
  2. set_policy_version_guc fails for some reason
  3. rollback_policy() returns Ok(None) which means nothing to roll back to.
  4. Function returns false.
  5. The engine is still running the new policy but cedarling.policy_version and cedarling.policy_history are stale.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

resolved in b6546fe

Handle the case where no previous engine version exists to roll back to,
ensuring users are notified when the engine and GUC state become
inconsistent.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
Update GUC registration to use `GucContext::Suset` for settings that
affect authorization logic, preventing unprivileged users from
bypassing security policies or changing enforcement modes.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
configuration functions

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
authorization

Introduce a `pg_test` that exercises the `cedarling_authorized` SQL
function with a real engine configuration. This covers the signed
path previously only tested at the Rust unit level.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
- Clear global cache on schema or mask configuration updates to prevent
  stale authorization decisions.
- Reject non-finite floats in SQL pushdown to avoid ambiguous predicate
  behavior.
- Update documentation for `cosign` verification, function parameter
  types, and SLSA metadata.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
haileyesus2433 and others added 2 commits June 4, 2026 14:12
Signed-off-by: Haileyesus Ayanaw <85413826+haileyesus2433@users.noreply.github.com>
Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
@haileyesus2433

Copy link
Copy Markdown
Contributor Author

Here is a review from Claude. Can you pls analyze each case?

cedarling_pg Review — Docs, Security, Tests

Reviewed the cedarling_pg changes on this branch vs main: documentation, the Rust/pgrx extension source, generated SQL, install script, and the #[pg_test] suite.

Overall: solid, security-conscious work — fail-closed defaults, parameterized SPI everywhere, careful pure-Rust quoting of identifiers/literals, and a candid security section in the docs. The findings below are the real gaps worth addressing before merge.

1. Documentation

The docs are high quality and largely accurate (GUC table, function inventory, PG matrix 13–18, asset/provenance names all verified against code). Defects:

🔴 CRITICAL

  • docs/cedarling/integrations/postgres.md:131-135 — the cosign verify-blob command will not run. It only passes --certificate-oidc-issuer-regexp and omits a certificate-identity matcher (--certificate-identity / --certificate-identity-regexp), which cosign verify-blob requires. The command errors out, so the recommended verification step fails. Compare the complete, correct form in docs/contribute/ci-cd/slsa-verification.md:720-724.

🟠 IMPORTANT

  • postgres.md:294,296 — wrong parameter type regclass vs oid. cedarling_validate_schema and cedarling_register_entity_map actually take oid, not regclass (sql/cedarling_pg--0.1.0.sql:372, :253). A bare table-name literal will not auto-cast to oid; callers must write 'mytable'::regclass::oid (as the code comment at src/policy/schema.rs:87 states). As documented, calls fail.
  • postgres.md:53 — non-callable example. The flow narrative shows cedarling_authorized_row(students, 'Read') (2 args), but every overload requires 3 args (record, action, context) with no SQL DEFAULT (sql/...sql:117-121). This contradicts the doc's own reference row at postgres.md:266.

🟡 MINOR

  • cedarling-pg artifact family is missing from the SLSA "Artifact Families" table (slsa-verification.md:781-791), even though cedarling-pg.intoto.jsonl is published (build-packages.yml:1880) and postgres.md:529 tells users it exists.
  • cedarling_mask_row parameter documented as row (postgres.md:305); actual name is row_json (sql/...sql:230). Breaks named-argument calls.
  • The instrumentation value of cedarling.mode (postgres.md:332) is listed but never explained; code defines it as "evaluate and log, still return the decision" (src/guc_config.rs:24-27).

2. Security

🔴 CRITICAL

None.

🟠 HIGH

H1 — Enforcement-affecting GUCs are Userset, so any role can weaken enforcement for its own session. src/guc_config.rs:300-352. cedarling.mode, cedarling.fail_mode, cedarling.strategy, and cedarling.where_partial_fallback are all registered GucContext::Userset. Any role with query access can run:

SET cedarling.mode = 'shadow';     -- finalize_decision() returns true unconditionally (authorized.rs:357)
SET cedarling.fail_mode = 'open';  -- finalize_error() returns true on any error (authorized.rs:366)

This neutralizes authorization for that session — directly impacting any RLS policy or SECURITY DEFINER view relying on cedarling_authorized*, since RLS predicates evaluate in the querying user's session. Fix: register these as GucContext::Suset (matching bootstrap_config, policy_version, mask_hash_salt at :449,459,467).

H2 — cedarling_set_mask_config and cedarling_register_entity_map are executable by PUBLIC. src/mask/mod.rs:34, src/resource/schema_map.rs:195. Only the four policy-lifecycle functions are REVOKEd (sql/cedarling_pg--0.1.0.sql:351-354). entity_map controls which Cedar entity type/id a row is evaluated as (schema_map.rs:39-67) — a security-relevant input to the decision — yet is mutable by anyone. An attacker can remove a masking rule or remap an entity to change authorization outcomes. Fix: REVOKE EXECUTE ... FROM PUBLIC on both functions and REVOKE INSERT/UPDATE/DELETE ON cedarling.mask_rules, cedarling.entity_map FROM PUBLIC. Grant writes only to the extension owner / admin role.

🟡 MEDIUM

  • M1 — where_partial_fallback = permit lowers an un-lowerable predicate to TRUE. Safe only when paired with a row-level cedarling_authorized* re-check, which nothing enforces. Default is deny (good); mitigated by fixing H1 (making it Suset). src/authz/where_clause.rs:90-93,494-499,645-663.
  • M2 — Cache not invalidated on entity_map/mask_rules changes. Only use_policy/rollback call clear_all (versions.rs:99,170). Tightening an entity mapping has no effect until TTL expiry (default 300s) — a stale-allow window. Cache key also omits current_user/search_path (defense-in-depth note for pooled deployments). src/authz/cache.rs:48-102.
  • M3 — lower_scalar_literal emits inf/-inf/nan/infinity verbatim into SQL. src/authz/where_clause.rs:277 (t.parse::<f64>().is_ok()). Source is admin-authored policy text (not attacker-controlled at query time) and the constrained alphabet prevents arbitrary SQL injection, but it silently produces an incorrect/erroring predicate, undermining pushdown correctness. Fix: reject non-finite floats (!value.is_finite()) or quote+cast.

🟢 LOW

  • L1/L2cedarling_use_policy, cedarling_register_policy_version, cedarling_diff_policies read arbitrary server-side file paths (versions.rs:66,27,204). Correctly REVOKEd from PUBLIC, so owner-only — appropriate. Paths are unvalidated (no allowlist/canonicalization); an over-broad grant later would become a file-read/SSRF primitive (bootstrap can reference remote URIs). Consider restricting to a configured directory.
  • L3 — Disabling JWT signature/status validation is only a WARN log (cedarling/src/jwt/mod.rs). Defaults are strict; the test bootstrap disables it (test-only). Ensure no shipped sample/docker-initdb bootstrap disables validation.
  • L4 — Cedar diagnostics and resource JSON may surface in the observability trace ring. Token bundle code correctly avoids echoing raw JWTs (tokens/bundle.rs:116-129). Ensure trace-reading functions are not granted to PUBLIC if traces can contain row data.

✅ Confirmed safe (reviewed)

  • WHERE-pushdown SQL injection: identifiers constrained to [A-Za-z0-9_] (parse_resource_field:256-264) and quoted via quote_ident_safe; string literals Cedar-unescaped then re-quoted via quote_literal_safe. String-state tracking in split_top_level/extract_clauses is correct. No raw policy text concatenated into SQL.
  • All SPI calls use parameter binding ($1, .into()) across mask/mod.rs, schema_map.rs, versions.rs, tokens/sql.rs, where_clause.rs:544.
  • NULL action errors (does not default to "Read"): authorized_row.rs:44-46, :302-304Nonefinalize_error(RequestInvalid) → fail-closed. The recent change is correct.
  • Cache key completeness: policy segment + tokens/principal + resource + action + context, each length-prefixed (cache.rs:69-85). Only Ok decisions cached; errors never cached; mutex poison → fail-closed.
  • Masking fail-closed: unknown mask_type → redact; Hash without salt → [HASH_SALT_REQUIRED] sentinel; condition_sql is rejected, never executed (mask/mod.rs:206-217). Salt GUC is Suset.
  • sanitize_table_filename does not exist in this codebase — no path-traversal sink found. File IO is reachable only through Suset/REVOKE-gated functions.

3. #[pg_test] Tests

pgrx idioms are correct: proper Spi::run / Spi::get_one / Spi::get_one_with_args, meaningful assertions, Drop-guards for cleanup, per-pid temp dirs, engine+cache reset between cases. No assert!(true) placeholders, no #[ignore]. The pure-Rust where-lowering unit suite is genuinely thorough.

However, coverage is skewed toward the unsigned path and fail-closed / no-engine artifacts.

🔴 CRITICAL gaps

  1. The signed / JWT authorization path is never exercised against a real engine. cedarling_authorized (multi-issuer, authorized.rs:191) is only ever called with no engine, in shadow, or with empty input. get_matching_policies_multi_issuer and the token-bundle bridge never run in a #[pg_test] with a loaded engine. Signed allow and deny are untested end-to-end.
  2. A real-engine grant (allow=true) is asserted in exactly one place — the unsigned RLS test (pg_test_rls_unsigned.rs). That is the only genuine allow-and-deny pair. cedarling_authorized_row is never tested returning a real engine true.

🟠 IMPORTANT gaps

  • cedarling_diff_policies has no #[pg_test] — only the pure text helpers are unit-tested (versions.rs:380-402). SQL-facing function, diff_mode dispatch, and file-path error branch untested.
  • GUC check-hook rejection untested — no test does SET cedarling.context = '[]' (or invalid tokens JSON) and asserts the SET errors (guc_config.rs:261-276).
  • cedarling_set_tokens / cedarling_current_tokens / cedarling_clear_tokens have zero #[pg_test] — the jsonb→GUC→jsonb roundtrip is never tested.
  • cedarling_mask_plan (mask/mod.rs:82) untested.
  • Rollback with no previous policy (cedarling_rollback_policy returning false, versions.rs:162) untested; only the happy path is.

🟡 MINOR gaps

  • NULL-action None arm of cedarling_authorized_row not directly tested (tests pass Some(...) or empty string).
  • sanitize_id_columns (schema_map.rs:139) has no malicious-input test (quotes/dots/-- in id column names).
  • No path-traversal test for the file-path functions (arbitrary-path read is reachable from SQL but untrusted-path handling is untested).
  • cedarling_status / observability counters never asserted from SQL.

Verdict

Test-code quality is good; authorization-decision coverage is incomplete. The suite proves the plumbing and the conservative SQL lowering well, but under-verifies the actual authorization decisions the extension exists to make — most importantly the signed/JWT path, which has no real-engine integration test at all.

Recommended fix order

  1. H1 — make mode/fail_mode/strategy/where_partial_fallback Suset. (highest impact)
  2. H2REVOKE mask/entity-map functions and tables from PUBLIC.
  3. Test CRITICAL gap — add a real-engine signed allow/deny integration #[pg_test].
  4. Docs CRITICAL — fix the cosign verify-blob command (quick).
  5. Docs IMPORTANT (regclassoid, 2-arg example) and remaining test/security mediums.

addressed applicable comments from e2e86cc - 0ab17e5

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
@olehbozhok

olehbozhok commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Re-check of remaining review items after fix batch (e2e86cc0ab17e5)

Re-verified all 13 items from the review. Summary:

✅ Fully addressed (9/13)

  • H1mode/fail_mode/strategy/where_partial_fallback all Suset
  • H2cedarling_set_mask_config + cedarling_register_entity_map REVOKEd from PUBLIC; mask_rules/entity_map tables REVOKEd INSERT/UPDATE/DELETE ✅
  • Test criticaltest_signed_authorized_allow_then_deny exercises JWT path (allow + deny) against a real engine ✅
  • All 6 doc defects corrected (cosign command, regclass→oid, 2-arg example, SLSA table, param name row_json, instrumentation docs) ✅

⬜ Remaining — 3 not addressed, 1 partial

1. JWT tokens via GUC — end-to-end untested (was: "token functions pg_test")

This is more important than "no pg_test for 3 thin wrappers."

The production JWT RLS path is:
SELECT cedarling_set_tokens('...'::jsonb);
CREATE POLICY rls ON t FOR SELECT
USING (cedarling_authorized_row_jwt(t, 'Read'));
SELECT * FROM t; -- RLS reads tokens from cedarling.tokens GUC

cedarling_authorized_row_jwt() (authorized_row.rs:301-310) passes None for token_bundle, so resolve_token_bundle() (authorized.rs:345-351) must read from the GUC. There is no alternative parameter.

The existing signed test (test_signed_authorized_allow_then_deny) passes tokens as an explicit argument to cedarling_authorized($1, $2, $3), which bypasses the GUC entirely. The GUC→engine bridge is only tested piecewise:

  • GUC check hook: unit-tested (validate.rs)
  • Token parsing: unit-tested (bundle.rs)
  • Engine authorize: unit-tested (bridge.rs)

But the end-to-end cedarling_set_tokens → cedarling_authorized_row_jwt → resolve_token_bundle → cedarling_authorized chain has no pg_test.

Severity: Medium. The components are individually correct, but the integration path is untested. Would be good to add before the first release.

2. cedarling_diff_policies — no pg_test

Owner-only utility (REVOKEd from PUBLIC). Rust diff logic is unit-tested (versions.rs:380-402). Missing: SQL integration test (file I/O error branch, diff_mode dispatch).

Severity: Low. The function is not in the auth path.

3. cedarling_mask_plan — no pg_test

Simple read-only SELECT from cedarling.mask_rules. Owner-only. The underlying table operations are exercised by other mask tests.

Severity: Low.

4. GUC check-hook rejection — no direct SET ... = 'invalid' pg_test

Validation logic (context_json_is_valid_object, tokens_json_is_valid) is unit-tested. Function-level rejection is integration-tested (test_input_validation_fail_closed asserts deny for context = '[]'). What's missing: a pg_test that runs SET cedarling.context = '[]' and asserts the SET errors.

Severity: Low. The check hook is a thin C-unwind bridge; the logic behind it is already verified.

Refactor the existing signed authorization test to extract helper
functions
for engine bootstrapping and token bundle generation. Added a new test
case that validates the full end-to-end integration of
`cedarling_authorized_row_jwt` reading tokens from the
`cedarling.tokens`
GUC, which mimics the production RLS pattern.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
diffing

- Add `test_guc_check_hooks_reject_invalid_input_at_set_time` to verify
  that GUC validation logic correctly rejects invalid JSON inputs via
  SQLSTATE 22023.
- Add `test_diff_policies_structural_lines_and_io_error` to validate
  both
  structural and line-based policy diffing modes and ensure file I/O
  errors are handled gracefully.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
@haileyesus2433

Copy link
Copy Markdown
Contributor Author

Re-check of remaining review items after fix batch (e2e86cc0ab17e5)

Re-verified all 13 items from the review. Summary:

✅ Fully addressed (9/13)

  • H1mode/fail_mode/strategy/where_partial_fallback all Suset
  • H2cedarling_set_mask_config + cedarling_register_entity_map REVOKEd from PUBLIC; mask_rules/entity_map tables REVOKEd INSERT/UPDATE/DELETE ✅
  • Test criticaltest_signed_authorized_allow_then_deny exercises JWT path (allow + deny) against a real engine ✅
  • All 6 doc defects corrected (cosign command, regclass→oid, 2-arg example, SLSA table, param name row_json, instrumentation docs) ✅

⬜ Remaining — 3 not addressed, 1 partial

1. JWT tokens via GUC — end-to-end untested (was: "token functions pg_test")

This is more important than "no pg_test for 3 thin wrappers."

The production JWT RLS path is: SELECT cedarling_set_tokens('...'::jsonb); CREATE POLICY rls ON t FOR SELECT USING (cedarling_authorized_row_jwt(t, 'Read')); SELECT * FROM t; -- RLS reads tokens from cedarling.tokens GUC

cedarling_authorized_row_jwt() (authorized_row.rs:301-310) passes None for token_bundle, so resolve_token_bundle() (authorized.rs:345-351) must read from the GUC. There is no alternative parameter.

The existing signed test (test_signed_authorized_allow_then_deny) passes tokens as an explicit argument to cedarling_authorized($1, $2, $3), which bypasses the GUC entirely. The GUC→engine bridge is only tested piecewise:

  • GUC check hook: unit-tested (validate.rs)
  • Token parsing: unit-tested (bundle.rs)
  • Engine authorize: unit-tested (bridge.rs)

But the end-to-end cedarling_set_tokens → cedarling_authorized_row_jwt → resolve_token_bundle → cedarling_authorized chain has no pg_test.

Severity: Medium. The components are individually correct, but the integration path is untested. Would be good to add before the first release.

2. cedarling_diff_policies — no pg_test

Owner-only utility (REVOKEd from PUBLIC). Rust diff logic is unit-tested (versions.rs:380-402). Missing: SQL integration test (file I/O error branch, diff_mode dispatch).

Severity: Low. The function is not in the auth path.

3. cedarling_mask_plan — no pg_test

Simple read-only SELECT from cedarling.mask_rules. Owner-only. The underlying table operations are exercised by other mask tests.

Severity: Low.

4. GUC check-hook rejection — no direct SET ... = 'invalid' pg_test

Validation logic (context_json_is_valid_object, tokens_json_is_valid) is unit-tested. Function-level rejection is integration-tested (test_input_validation_fail_closed asserts deny for context = '[]'). What's missing: a pg_test that runs SET cedarling.context = '[]' and asserts the SET errors.

Severity: Low. The check hook is a thin C-unwind bridge; the logic behind it is already verified.

resolved from 6b3a339 - e180532

dagregi
dagregi previously approved these changes Jun 9, 2026
moabu
moabu previously approved these changes Jun 9, 2026
olehbozhok
olehbozhok previously approved these changes Jun 9, 2026

@olehbozhok olehbozhok left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LOW / defense-in-depth: trace & observability functions are PUBLIC-executable over a per-backend ring buffer

cedarling_last_trace(), cedarling_recent_traces(int), cedarling_explain(text, text), and cedarling_status() are not
REVOKEd from PUBLIC (sql/cedarling_pg--0.1.0.sql:196,208,241,311), unlike the policy-lifecycle and entity/mask
writers.

The trace ring is per-backend (observability/trace.rs), so it persists across sessions that share the same backend
process — i.e. in any connection-pooled deployment (PgBouncer in session/transaction pooling, app-side pools). A
low-privilege role can therefore call cedarling_recent_traces() and read trace entries produced by other sessions that
previously ran on that backend.

What's exposed per entry: resource_type, resource_id (the row's primary-key value), action, principal_id, and
diag_errors. This is authorization metadata, not row contents — so impact is limited — but resource_id can be a
sensitive identifier, and this crosses a session boundary the caller shouldn't see across.

Note this is metadata-only and gated behind connection pooling — hence LOW, not a merge blocker. Raising it for an
explicit decision.

Suggested fix — pick one:

  1. Restrict the readers (preferred if traces aren't needed by app roles):
    REVOKE EXECUTE ON FUNCTION cedarling_last_trace() FROM PUBLIC;
    REVOKE EXECUTE ON FUNCTION cedarling_recent_traces(int) FROM PUBLIC;
    REVOKE EXECUTE ON FUNCTION cedarling_explain(text, text) FROM PUBLIC;
    REVOKE EXECUTE ON FUNCTION cedarling_status() FROM PUBLIC;
    -- GRANT EXECUTE ... TO <observability_role>; -- as needed
  2. Or document the pooling caveat in docs/cedarling/integrations/postgres.md (security section) — state that trace
    readers may surface cross-session metadata under shared backends and should be granted only to trusted observability
    roles.

cedarling_explain is borderline: it re-runs authorization on caller-supplied input and returns the caller's own
diagnostics, so it's less of a cross-session leak — but it still reads the shared ring indirectly via push_trace, so
include it in whichever option you choose.

Everything else in the security pass came back clean (H1/H2 confirmed fixed, SQL fully parameterized, pushdown
fail-closed). This is the only residual.

@tareknaser tareknaser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you! I only have one main comment left then I think we're ready to merge

return None;
}

let when_clauses = extract_clauses(&meta.source, "when")?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think there's a correctness issue here. lower_policy_to_sql only looks at when/unless clauses and ignores constraints in the policy head.

For example, permit(principal, action, resource == Doc::"42") lowers to TRUE, even though the policy only applies to a single resource. Similarly, permit(principal, action, resource == Doc::"42") when { resource.public == true } lowers to "public" = TRUE, dropping the resource constraint entirely.

From what I can tell, matching_policies_for_table filters policies by entity type rather than specific scope constraints so scoped policies can reach this code path and have their principal/resource restrictions discarded during SQL lowering.

That means cedarling_where can produce predicates that match more rows than the underlying Cedar policy allows.

},
};

let previous = match engine::use_policy(&resolved.bootstrap_path) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The GUC failure handling looks good now. One thing that still seems worth documenting is that the policy swap is per-backend and not transactional.

engine::use_policy updates the Rust static ENGINE immediately so a ROLLBACK will undo the catalog changes and set_config but the backend will keep using the new policy. Also, because the engine is process-local, cedarling_use_policy only affects the current backend. other connections keep their own engine instance.

The current comment in catalog.rs ("global policy swap") and the wording in postgres.md read as if this is cluster-wide. It would be helpful to clarify that the swap is per-backend and non-transactional and that cluster-wide changes should be done by updating cedarling.bootstrap_config and reconnecting rather than relying on cedarling_use_policy to propagate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

resolved in 20a8dd8

Ensure `lower_policy_to_sql` does not drop principal or resource
identity restrictions by forcing such policies to the unhandled-residual
path. This prevents over-permitting when the SQL predicate cannot
enforce specific entity ID constraints.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
extension owner

Revoke EXECUTE permissions from PUBLIC for all observability functions
to
prevent potential cross-session data leakage when using connection
pooling. Grant these permissions explicitly to monitoring roles as
required.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
behavior

Clarify that `cedarling_use_policy` and `cedarling_rollback_policy`
mutate process-local state. Explicitly note that while catalog updates
are transactional, the engine swap is not, and advise using
`cedarling.bootstrap_config` for cluster-wide changes.

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
@haileyesus2433 haileyesus2433 dismissed stale reviews from dagregi, moabu, and olehbozhok via 20a8dd8 June 10, 2026 17:59
@haileyesus2433

Copy link
Copy Markdown
Contributor Author

LOW / defense-in-depth: trace & observability functions are PUBLIC-executable over a per-backend ring buffer

cedarling_last_trace(), cedarling_recent_traces(int), cedarling_explain(text, text), and cedarling_status() are not REVOKEd from PUBLIC (sql/cedarling_pg--0.1.0.sql:196,208,241,311), unlike the policy-lifecycle and entity/mask writers.

The trace ring is per-backend (observability/trace.rs), so it persists across sessions that share the same backend process — i.e. in any connection-pooled deployment (PgBouncer in session/transaction pooling, app-side pools). A low-privilege role can therefore call cedarling_recent_traces() and read trace entries produced by other sessions that previously ran on that backend.

What's exposed per entry: resource_type, resource_id (the row's primary-key value), action, principal_id, and diag_errors. This is authorization metadata, not row contents — so impact is limited — but resource_id can be a sensitive identifier, and this crosses a session boundary the caller shouldn't see across.

Note this is metadata-only and gated behind connection pooling — hence LOW, not a merge blocker. Raising it for an explicit decision.

Suggested fix — pick one:

  1. Restrict the readers (preferred if traces aren't needed by app roles):
    REVOKE EXECUTE ON FUNCTION cedarling_last_trace() FROM PUBLIC;
    REVOKE EXECUTE ON FUNCTION cedarling_recent_traces(int) FROM PUBLIC;
    REVOKE EXECUTE ON FUNCTION cedarling_explain(text, text) FROM PUBLIC;
    REVOKE EXECUTE ON FUNCTION cedarling_status() FROM PUBLIC;
    -- GRANT EXECUTE ... TO <observability_role>; -- as needed
  2. Or document the pooling caveat in docs/cedarling/integrations/postgres.md (security section) — state that trace
    readers may surface cross-session metadata under shared backends and should be granted only to trusted observability
    roles.

cedarling_explain is borderline: it re-runs authorization on caller-supplied input and returns the caller's own diagnostics, so it's less of a cross-session leak — but it still reads the shared ring indirectly via push_trace, so include it in whichever option you choose.

Everything else in the security pass came back clean (H1/H2 confirmed fixed, SQL fully parameterized, pushdown fail-closed). This is the only residual.

addressed the remaining comments in here 2f3e7da

Signed-off-by: haileyesus2433 <haileyesusbe@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-documentation Documentation needs to change as part of issue or PR comp-docs Touching folder /docs comp-jans-cedarling Touching folder /jans-cedarling kind-feature Issue or PR is a new feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(jans-cedarling): Cedarling PostgreSQL Extension

6 participants