refactor(daemon): make the token permission check assertable, and cover it - #86
Merged
Merged
Conversation
…er it
Every mutant the gate generated for check_token_permissions survived, and
the reason was structural rather than an oversight in the tests: the
function's only output was a tracing::warn!. There was no value to assert,
so `!path.exists()`, the `& 0o777` mask and the `!= 0o600` comparison could
all be flipped without any test noticing.
This splits the decision from the reporting. token_permissions() returns
Absent / Secure / Insecure{mode} / Unknown, and check_token_permissions()
keeps its signature and simply warns on Insecure, so the single caller in
commands/server.rs is untouched. Behaviour is unchanged: the same condition
produces the same warning.
Unknown is a deliberate fourth variant rather than folding a failed
metadata read into Secure. The original silently skipped the check when
metadata could not be read, which is the right call - an unreadable file is
not evidence of a bad mode - but calling that "secure" would be a lie in
the type.
All 7 mutants were hand-applied and the suite confirmed to fail: 7 killed,
0 survived.
The mask mutations are the ones worth noting. A file created 0644 has a raw
mode of 0o100644 including the file-type bits: masking gives 0o644, `^`
gives 0o100133 and `|` gives 0o100777. All three are "insecure", so a test
asserting only the variant would miss both mutations. The tests assert the
mode value that comes back, which is what separates them.
validate_bearer gains coverage in the same pass, including the cases that
distinguish a prefix check from an equality: a token that is a prefix of
the expected one, a token that extends it, and a header whose prefix is
"Bearer" without the trailing space.
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.
Every mutant the gate generated for
check_token_permissionssurvived, and the reason was structural rather than an oversight in the tests: the function's only output was atracing::warn!. There was no value to assert, so!path.exists(), the& 0o777mask and the!= 0o600comparison could all be flipped without a single test noticing.The split
token_permissions()returns the decision:check_token_permissions()keeps its signature and simply warns onInsecure, so the single caller incommands/server.rsis untouched and the same condition produces the same warning.Unknownis a deliberate fourth variant rather than folding a failed metadata read intoSecure. The original silently skipped the check when metadata could not be read — that is the right call, since an unreadable file is not evidence of a bad mode — but calling it "secure" would be a lie in the type.Mutants
All 7 hand-applied and confirmed to fail the suite: 7 killed, 0 survived.
The mask mutations are the ones worth naming. A file created
0644has a raw mode of0o100644including the file-type bits:mode & 0o7770o644mode ^ 0o7770o100133mode | 0o7770o100777All three are "insecure", so a test asserting only the variant would miss both mutations. The tests assert the mode value that comes back, which is what separates them.
validate_bearergains coverage in the same pass, including the cases that distinguish a prefix check from an equality: a token that is a prefix of the expected one, a token that extends it, and a header whose prefix is"Bearer"without the trailing space.cargo fmt --check,cargo clippy --all-targets -- -D warningsand the full 872-test lib suite are clean.