fix(compute-gateway): mask every result shape, not just nodes - #1396
Merged
Conversation
mdheller
force-pushed
the
fix/masking-covers-all-result-shapes
branch
from
August 4, 2026 14:59
b451d8b to
96bef83
Compare
Closes a LIVE hole. The masking PDP went on in #1387, and `_iter_records` walked `data["nodes"]` and nothing else — so a result shaped as rows / table / edges was returned UNMASKED while the decision output reported the policy active. `rows` is a shape this estate actually emits (hellgraph + compute-gateway both do), so personal data was being served in cleartext on those paths with masking nominally enforced. I pinned that limit with a test rather than fixing it when the policy landed. That was the wrong call for something already enforcing in production. The walker is now shape-agnostic and depth-bounded: every nested dict is a candidate record. An allowlist of container keys only moves the hole to the next shape somebody adds. This is safe because _mask_record rewrites ONLY keys named in the policy's mask_fields — a dict with no configured field comes back untouched — so over-walking does nothing while under-walking serves personal data. test_over_walking_is_harmless asserts a payload with no configured field is returned byte-identical, so widening coverage cannot corrupt unrelated envelopes. output.data itself is excluded: it is the envelope, not a record, and masking its top level would rewrite container keys that happen to collide with a field name. Tests: every shape (nodes/rows/edges/records) asserted to leak nothing; nested payloads reached; over-walking harmless; READ_KINDS unchanged so widening the walker does not widen the kinds governed. 252 pass.
mdheller
force-pushed
the
fix/masking-covers-all-result-shapes
branch
from
August 4, 2026 15:47
96bef83 to
0cd23cc
Compare
mdheller
enabled auto-merge (squash)
August 4, 2026 16:39
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.
rowsis a shape this estate actually emits — both hellgraph-service and compute-gateway produce it. So personal data was being served in cleartext on those paths with masking nominally enforced.I pinned that limit with a test instead of fixing it when the policy landed. Wrong call for something already enforcing in production.
The fix
The walker is now shape-agnostic and depth-bounded — every nested dict is a candidate record. An allowlist of container keys only moves the hole to the next shape someone adds.
This is safe because
_mask_recordrewrites only keys named inmask_fields. A dict with no configured field comes back untouched. So:Fail toward the former.
output.dataitself is excluded: it's the envelope, not a record, and masking its top level would rewrite container keys that collide with a field name.Tests
test_every_result_shape_is_masked_not_just_nodestest_nested_payloads_are_reachedresult.page.itemsis still maskedtest_over_walking_is_harmlesstest_non_read_kinds_still_untouched252 tests pass.