Conversation
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Teryl Taylor <terylt@ibm.com>
araujof
marked this pull request as ready for review
June 16, 2026 01:45
Config: from_config now rejects unknown keys (e.g. on_errr typo) with
BuildError::ConfigShape instead of silently defaulting.
Tests: add concurrent-evaluation unit test (shared resolver, one cached
program), plus end-to-end integration tests for a missing expr at
request time and for reading meta.entity_name through a cel: step.
Docs: canonicalize the cel: { expr } step form (lib.rs, step.rs) noting
the call form is also accepted; add a concrete non-boolean-result
example (CEL null vs false); add CEL-vs-Cedar guidance and a
synchronous-by-design note; clarify on_error: allow does not enforce PDP
layering; document custom-function set ownership; trim Cargo.toml header
redundant with lib.rs; refresh the PdpRouter backend list to include cel.
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
araujof
added a commit
that referenced
this pull request
Jun 18, 2026
Revert formatting-only changes to apl-pdp-cel / apl-pdp-cedar-direct / apl-cpex files that a crate-wide `cargo fmt` reformatted (pre-existing drift from #68) and `git add -A` swept into earlier commits. Keeps this PR focused on the Valkey session store; those files now match main exactly. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
araujof
added a commit
that referenced
this pull request
Jun 18, 2026
…labels) (#74) * docs: add Valkey session store requirements and implementation plan Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * feat(apl-cpex): make SessionStore fallible and fail closed on store errors U1+U2: SessionStore trait methods now return Result with a crate-local SessionStoreError. MemorySessionStore adapts (infallible -> Ok). The CMF invoker (for_request, persist_session) and route_handler propagate: - a load error fails the request closed before any decision (R5) - an append error flips the outcome to Deny with merge precedence (Allow+Err -> session.persist_failed; Deny+Err -> keep policy violation), with a distinguished alarm (R18) Sessionless traffic never touches the store. Adds fail-closed tests (AE1, AE6) plus a sessionless carve-out. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * feat(apl-cpex): config-driven SessionStore selection via factory U3: Add SessionStoreFactory (mirrors PdpFactory) and a global.apl.session_store config block. The visitor builds the store during visit_global and swaps its own (now RwLock-held) session_store field before visit_route clones it into handlers — no per-request indirection. Default MemorySessionStore stays active when no block is present (R3). AplOptions gains session_store_factories; all struct-literal sites updated. Adds config-selection and unknown-kind tests (AE3, AE5). Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * feat(valkey): add Valkey-backed SessionStore crate + feature-gated FFI wiring U4/U5/U6: New apl-session-valkey crate (redis-rs 1.x + deadpool-redis over rustls, no openssl), excluded from default-members. ValkeySessionStore stores labels as a Redis SET keyed by taint:v1:<sha256(session_id)>; append is a single atomic SADD(+EXPIRE) pipeline (R16); load maps key-miss->Ok(empty) and backend/timeout->Err (R5/R15); sliding-TTL refresh on load is fail-open (R7). Config parsing enforces TLS for non-localhost endpoints (R10), commits safe timeout defaults, and warns when TTL < session lifetime (R17). Wired into cpex-ffi behind the optional 'valkey' feature; default build links no Valkey object code (R13). Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * test(valkey): container-backed integration tests + security hardening U7: Integration tests (testcontainers valkey, or VALKEY_TEST_URL escape hatch; #[ignore]d by default, CI-gated via REQUIRE_VALKEY_TESTS) covering cross-node union (R16/AE4), unknown->empty (R15), WRONGTYPE->fail-closed and unreachable-> fail-closed (R5), and TTL set+refresh (R7/AE2). deploy/valkey-compose.yml runs a noeviction-configured Valkey for local dev. Also hardens config.rs per security review: connection_url() percent-encodes credentials via the url crate and always reflects tls_enabled() in the scheme; rejects the tls:true + plaintext redis:// contradiction; redacts credentials from all error/log output. Adds regression tests. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * docs(valkey): operator runbook for the Valkey session store U8: documents the operator-owned controls the backend depends on but cannot enforce — noeviction (R9) + monitoring, TLS/mTLS + least-privilege ACL (R10), the TTL soundness rule and refresh-failure alarm (R8/R17), the single-endpoint primary-only topology and accepted fail-closed availability tradeoff (with the sessionless carve-out), the v0 no-live-reload limitation, the alarm catalog, and local-dev setup. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * chore: drop unrelated rustfmt churn swept in by cargo fmt -p Revert formatting-only changes to apl-pdp-cel / apl-pdp-cedar-direct / apl-cpex files that a crate-wide `cargo fmt` reformatted (pre-existing drift from #68) and `git add -A` swept into earlier commits. Keeps this PR focused on the Valkey session store; those files now match main exactly. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * fix(valkey): wire connect_timeout, drop dead max_retries knob, harden TLS feature Addresses code-review findings: - connect_timeout_ms now bounds connection acquisition (distinct from the per-command timeout) instead of being parsed-but-ignored. - Remove the max_retries config knob: no retry/circuit-breaker exists in v0 (deferred follow-up), so the store fails closed on first error — config no longer advertises behavior the code lacks. - Forward tokio-rustls-comp through deadpool-redis explicitly so the rediss:// TLS path doesn't depend on incidental feature unification. - Add Deny+append-failure regression test: the original policy violation is preserved (not overwritten by session.persist_failed) per R18 merge rules. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * style(apl-cpex): drop needless borrows in strip_non_dsl_keys (clippy) Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * chore: mark Valkey session store plan completed Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * docs: fix mermaid parse error in plan config-selection diagram ASCII '->' inside sequence-diagram message text was tokenized as an arrow. Replace with 'yields' / Unicode → (matching the requirements-doc diagrams). Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * docs: fix real mermaid culprit — semicolon in sequence message text Mermaid sequence message text terminates at ';', so 'yields ...; swap ...' cut the message early and the parser choked on the next line. Replace the semicolon with ', then' and drop the Arc<dyn ...> angle brackets. (The earlier '->' fix was needed too but not the actual break.) Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * docs,valkey: document persistence durability + harden credential config Addresses the PR #74 review (terylt). Substantive ask — persistence/durability as a security contract: - Runbook gains a "Persistence and durability" section (§5): the label keyspace is a system-of-record, not a cache. A SADD acked then lost to a crash before fsync returns Ok(empty) on the next read (not an error), so fail-closed never trips — a silent downgrade invisible to all alarms. Documents the three fsync options, a recommended AOF baseline, and the async-replication failover interaction with the topology section. - Adds R19 to the requirements doc, peer to the R9 noeviction contract. Doc fix found while verifying: - The runbook claimed a CONFIG GET maxmemory-policy self-check that does not exist in code (the pool is lazy and never dials at config-load). Corrected §2 to present noeviction purely as operator contract; the actual self-check (noeviction + persistence) is deferred to #76. Code nits — hard-error ambiguous credential config at load, consistent with the existing tls:true + redis:// rejection: - `username` without `password` is rejected (previously dropped silently, connecting as the default user). - A full URL endpoint combined with separate `username`/`password` fields is rejected (those fields are ignored for URL endpoints). - Bare host:port credential application now applies the username when either credential is present; a lone password (default-user AUTH) stays valid. Adds regression tests for all three. apl-session-valkey unit tests green, clippy clean, cpex-ffi --features valkey builds. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * fix(apl): honor flat `session_store` key + warn at non-global scope `apl_subblock` lifts APL terms written directly on a section (no `apl:` wrapper) into a synthetic block, but only for the keys in FLAT_APL_KEYS. `pdp` was listed; `session_store` was not — so a flat `global.session_store` block was silently dropped, while `global.pdp` and the `apl:`-wrapped `global.apl.session_store` both worked. Asymmetric and a silent-config footgun. - Add `session_store` to FLAT_APL_KEYS so the flat form is honored, symmetric with `pdp` and with the wrapped form. - Introduce GLOBAL_ONLY_NON_DSL_KEYS (`pdp`, `session_store`) as the single source of truth for the keys that are CPEX wiring (acted on only by visit_global) and stripped before policy compilation. strip_non_dsl_keys now iterates it. - Generalize warn_if_pdp_at_nonglobal_scope -> warn_if_global_only_key_at_nonglobal_scope so a `session_store:` written at route/default/policy-bundle scope (where it is inert) is flagged, the same way `pdp:` already is — closing the new silent-no-op the flat key would otherwise introduce. Adds tests: flat session_store is collected by apl_subblock; the renamed warning helper is a safe no-op for both keys. apl-cpex tests green, clippy clean, cpex-ffi --features valkey builds. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> --------- Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for CEL (Common Expression Language) as a policy decision backend in APL. Authors can write inline boolean predicates over the common attribute vocabulary, evaluated through the existing
PdpResolverseam alongside Cedar, OPA, and AuthZen. No changes to APL evaluation semantics.Usage
Changes
New crate
apl-pdp-cel(mirrorsapl-pdp-cedar-direct):resolver.rs:CelResolverimplementsPdpResolver. Holds a compile-once cache keyed by expression source, so each distinctexprcompiles a single time.trueallows,falsedenies. Compile errors, undeclared-variable references, and non-boolean results are fail-closed (deny), with anon_error: allowconfig override.activation.rs: maps APL's flat dotted-keyAttributeBag(subject.id,delegation.depth,session.labels) into nested CEL maps so authors use natural field selection.StringSetmaps to a CEL list (enablesinand comprehension macros). Namespace wins on leaf/prefix collisions. Author-suppliedcel:args (resource,context) surface as variables, with bag values taking precedence.factory.rs:CelPdpFactory(kind = "cel") lets the apl-cpex visitor build the resolver from unified-config YAML.apl-core:
step.rs: adds first-classPdpDialect::Celandfrom_key("cel").parser.rs: recognizes thecel:step key withon_allow/on_denyreactions.Workspace:
apl-pdp-celadded tomembersanddefault-members.Crate
cel = "0.13"(clarkmcc, MIT). Sync evaluation, comprehension macros (has,all,exists,map,filter), custom functions. No static type checker, so validation is compile-time parse plus fail-closed runtime.Tests
in/has()/exists()macros, compile-cache reuse, fail-closed andon_error: allowpaths, config parsing.PluginManagerfrom YAML (kind: cel) through visitor, factory, resolver, and the allow/deny split, proving zero-glue operator wiring.cel:parse).cargo build --workspaceclean;apl-pdp-celclippy-clean.Notes
In the future, we could explore inline
cel("...")predicates inside rule conditions (when:/require/redact), reusing the same activation mapping.