Fix four Wasm-boundary marshalling and state bugs - #17
Merged
Conversation
Several exports in the Wasm module answer with a bare ERROR:<message> string instead of the JSON envelope. CedarEngine fed that straight to ObjectMapper.readValue, so a rejected payload surfaced as "Failed to parse cache response" — or, worse, as "Failed to serialize policy set" — with the module's actual complaint buried in a JsonParseException. That is why the schema-cache and template-link marshalling bugs went unnoticed: both fail this way, and neither error said anything useful. Check for the prefix before parsing and raise CedarException carrying the module's own message. CedarRawEngine deliberately does not go through this path, so raw() keeps returning the prefixed string verbatim.
cedar_preparse_schema parses its argument as JSON, and the Cedar variant of the FFI schema type is an untagged JSON string. cacheSchema passed the raw Cedar text straight through, so the module always answered "expected value at line 1 column 1" and caching a Cedar-format schema could never succeed. JSON-format schemas were unaffected. Encode the text for the CEDAR branch, mirroring what serializeSchema already does for the non-cached path. This also restores the schema mitigation on the cached authorization path: a cached Cedar schema now informs context parsing, so a record carrying the reserved __entity key is rejected instead of being read as an entity reference.
The FFI declares link values as HashMap<SlotId, EntityUid>, i.e.
{"?principal": {"type": ..., "id": ...}}. TemplateLink serialized a list of
{slot, value} pairs instead, so the module rejected any policy set carrying
a template link with "invalid type: sequence, expected a map". Template
linking has never worked; nothing caught it because templates had no test
coverage.
Serialize the map form and keep linkValues() as the public list API.
Duplicate slots now fail at construction: the FFI keys by slot and rejects
duplicate keys, so a repeated slot is always an error and the caller should
hear about it there rather than mid-serialization.
Drops the @JsonCreator annotations, which described the old list shape and
were never exercised -- nothing in the codebase deserializes TemplateLink,
only responses are read back.
The FFI's stateful authorization call declares deny_unknown_fields and only accepts preparsedSchemaName -- there is no inline schema field -- so isAuthorizedCached cannot forward request.schema(). It dropped it without a word, which meant a caller who attached a schema specifically to get schema-directed context parsing silently got none. That is the documented mitigation for the reserved-key type confusion, so losing it quietly is a security gap, not just a surprise. Throw IllegalArgumentException when a request carries a schema and no schemaId was given, naming cacheSchema() and the four-argument overload as the supported route. The three-argument overload delegates here, so both are covered.
Preparsed policy sets and schemas live in a thread_local map inside the Wasm instance, and each CedarEngine owns its own instance. Caching on an engine borrowed from CedarEnginePool therefore reached only that one engine, and every other engine answered "preparsed policy set 'X' not found" — a DENY with isSuccess() false. It failed closed, but a caller reading only decision() saw unexplained denials, and the pool-plus-cache pattern the README recommends could not work. Add cachePolicySet/cacheSchema on the pool. A Wasm instance is not thread-safe, so the ops are not pushed eagerly across engines that may be on loan; they are recorded and replayed onto each engine as it is borrowed, which means mutation only ever happens on the thread that owns the engine. Engines created lazily after the caching call pick the ops up on first borrow. Each op is applied once immediately against a borrowed engine, so an invalid policy set or schema raises at the caching call rather than failing every later borrow. The per-engine scope of CedarEngine.cachePolicySet is unchanged and still intentional; the pool API is what spans engines.
Covers caching a schema by id, why the cached path rejects an inline schema, and caching on the pool rather than on a borrowed engine. Also shortens the comments added across this branch.
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
1 participant
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.
Four bugs in how cedar4j marshals data across the Wasm/JSON boundary, and in how it handles state that lives inside a Wasm instance. Found while auditing the July 2026 CedarJava CVEs — none of these are those CVEs. Two are outright broken features, one is a silent security gap, one is a footgun in the pooling pattern the Readme recommends.
Each commit is a test written first, then the fix.
Surface Wasm
ERROR:responses asCedarExceptionSeveral exports answer with a bare
ERROR:<message>string instead of the JSON envelope.CedarEnginefed that straight to Jackson, so a rejected payload surfaced asFailed to parse cache response— orFailed to serialize policy set— with the real complaint buried in aJsonParseException. That is why the next two bugs went unnoticed.CedarRawEngineis unchanged, soraw()still returns the prefixed string verbatim.Fix
cacheSchemafor Cedar-format schemascedar_preparse_schemaparses its argument as JSON, and the Cedar variant of the FFI schema type is an untagged JSON string. We passed raw Cedar text, so the module always answeredexpected value at line 1 column 1and caching a Cedar-format schema could never succeed. JSON-format schemas were unaffected.This also restores the schema mitigation on the cached path: a cached Cedar schema now informs context parsing, so a record carrying the reserved
__entitykey is rejected instead of being read as an entity reference.Serialize template link values as a map keyed by slot
The FFI declares link values as
HashMap<SlotId, EntityUid>. We serialized a list of{slot, value}pairs, so the module rejected any policy set carrying a template link withinvalid type: sequence, expected a map. Template linking has never worked — templates had no test coverage.linkValues()keeps returning the list; only the wire shape changes. Duplicate slots now fail at construction, since the FFI keys by slot and rejects duplicate keys.Reject an inline schema on the cached authorization path
The stateful FFI call declares
deny_unknown_fieldsand only acceptspreparsedSchemaName, soisAuthorizedCachedcannot forwardrequest.schema(). It dropped it silently, which meant a caller who attached a schema specifically to get schema-directed context parsing got none. Now throws, namingcacheSchema()and the four-argument overload.Add pool-level policy set and schema caching
Preparsed caches live in a
thread_localmap inside the Wasm instance, and eachCedarEngineowns its own. Caching on a borrowed engine reached only that engine; every other one answeredpreparsed policy set 'X' not found. Fails closed, but a caller reading onlydecision()saw unexplained denials.CedarEnginePool.cachePolicySet/cacheSchemarecord the op and replay it onto each engine as it is borrowed — a Wasm instance is not thread-safe, so mutation only ever happens on the thread that owns the engine. Engines created later pick the ops up on first borrow. Each op is applied once up front so an invalid payload fails at the caching call.The per-engine scope of
CedarEngine.cachePolicySetis unchanged and still intentional.Verification
mvn verify— 59,864 tests, 0 failures. Both previously-broken features confirmed working end to end: a template-linked policy produces an ALLOW naming the linked policy id, and a cached Cedar schema actually rejects the__entityescape.