Repository navigation
Group no longer prints its grant when it is dumped or encoded - #12
Merged
Merged
Conversation
The org's grant is a bearer credential a distributor accepts, and Group held it in a public property with nothing to redact it: print_r, var_dump and a json-encoded log context all printed it, the last including a Group nested in the context a logger encodes. macula-php#11. __debugInfo() and JsonSerializable now name the grant as present rather than printing it, which is what macula-py's Group does with repr=False and what macula-dotnet's UcanPresentation does with a private PrintMembers. options() is untouched: what reaches publishGroup and subscribeGroup still carries the grant, and the redaction is on the way to a log, not on the way to the wire. Three ways of reading an object are outside what a PHP class can redact, so the class doc names them instead of leaving them to be assumed away: an (array) cast and var_export() ignore __debugInfo and expose every property including the private ones, and serialize() writes what a caller asked to keep. The test pins all three so that a PHP which ever gains a hook for them fails the test rather than silently leaving the doc wrong. The new test was run against the pre-fix Group.php first and failed there on print_r showing the grant. Offline suite: 49 tests, 231 assertions, green.
…unasserted Two required changes from Saturnus, both text and test. The redaction itself it found correct, and it probed PHP 8.5 to confirm which paths honour __debugInfo. 1. The class doc said three ways of reading an object were outside the class's control. It was wrong twice over. More paths leak than the three named: get_object_vars, a foreach, an IDE step debugger (DBGp reads the property table), Symfony's VarDumper whose dump() and dd() start from an (array) cast and add __debugInfo's keys alongside the real ones, and PHPUnit's own exporter, which reports a failed assertEquals/assertSame on a Group through (array) and so prints the grant into the CI log. And serialize() is not a PHP limit at all: __serialize() exists, so keeping the grant there is this class's choice, and a persisted group working when it comes back is worth more than hiding it from the caller who asked to store it. The doc, the __debugInfo doc, the CHANGELOG and the test's comments now say what is redacted, what is not, and which of the latter is a choice. "every debugger" was false and is gone. The three positive assertions are phrased as "not redacted by this class", which is what they show. 2. Nothing asserted the grant still reaches the wire. A later refactor routing options() through __debugInfo would have stayed green here until a live run. options()['ucan'] is now asserted to be the grant, and a group with none to carry no key. Also covered: nested var_dump and debug_zval_dump in the redacted set, and get_object_vars in the not-redacted one. Offline suite: 49 tests, 234 assertions, green.
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.
A CI vehicle for the sha that
mainis about to fast-forward to, not the change itself: this repo's workflows trigger onmainand on pull requests, so a branch push alone runs neither, and the exact sha wantsci.ymlandsealed-interop.ymlgreen beforemainmoves. Once this is green,mainis fast-forwarded to542e59dand the branch deleted; GitHub reports this PR as merged by itself at that point, with no merge command.What it changes
Group'sucanwas apublic readonlyproperty with nothing to redact it, soprint_r,var_dump,json_encode($group)andjson_encode(['group' => $group])all printed the org's grant — a bearer credential a distributor accepts. The same leak macula-dotnet#15 fixed through a synthesizedToString, reached here by property reflection.__debugInfo()andJsonSerializablenow name the grant as present instead of printing it, which is what macula-py'sGroupdoes withrepr=False.options()is untouched: the grant still reachespublishGroupandsubscribeGroup, and the test asserts it, so a later refactor routingoptions()through__debugInfofails here rather than on a live run.What a PHP class cannot redact is now stated rather than implied: anything that reads the property table — an
(array)cast,get_object_vars, aforeach, an IDE step debugger, Symfony's VarDumper, and PHPUnit's own exporter, which prints the grant into the CI log when a comparison on aGroupfails.serialize()keeps it too, as this class's choice:__serialize()could redact it, and a group persisted to a queue or a cache is meant to work when it comes back.Evidence
Group.phpfirst and failed there onprint_rshowing the grant.Refs macula-php#11
🤖 Generated with Claude Code