Conversation
…ling `normalize_public_safe_field_name` is the single decision point in front of exact field classification, and the suffix families and the `raw` / `raw_*` rule only exist because it splits camelCase: `reviewToken` folds to `reviewtoken`, which belongs to no family, so without the boundary renaming a field to camelCase walks out of the rule untouched. No test held that equivalence, and the function had no direct reference in `tests/`. The set pins seven spellings of one family member onto one verdict, five suffix-shaped names that only the split classifies, the raw rule's camelCase reach, seven benign camelCase keys the boundary must not sweep up, and that the rejection names the caller's own key through nested mappings and lists. Production code is unchanged. Signed-off-by: JunZ-Leo <100498253+JunZ-Leo@users.noreply.github.com>
This branch has not been deployed
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.
Goal And Delivered Outcome
Goal/source and gap:
normalize_public_safe_field_nameis the single decision point in front of exact field-name classification insidevalidate_public_safe_value, and it had zero references intests/while having two production callers (loopx/control_plane/runtime/public_safety.pyitself andloopx/capabilities/reliability_diagnostics/envelope.py). Its own docstring states the claim — "Field classification is exact after case, separator, and camelCase normalization" — and nothing held it.What that costs concretely: the suffix families (
_token,_secret,_password,_credential,_credentials) and theraw/raw_*rule only match because the boundary splits camelCase.reviewTokenfolds toreviewtoken, which is in no family and ends in none of those suffixes, so renaming a field to camelCase would walk out of the rule untouched. Measured: with the split removed,reviewToken,rotateSecret,viewerPassword,handoffCredentialandnoteTokenall become acceptable names.Observable before → after, with the validation row that proves it: before, no test asserted that
accessTokenandaccess_tokenreach the same verdict; after, 21 assertions do, and five mutations against the production function each fail a distinct subset (seeregression_parity). No behaviour changes.Issue/task and intended base: no board item; this is the same shape as test(extensions): pin what a capped provider run returns and stops #4924 and test(turn-driver): pin the host-failure retry ceiling and unmapped policies #4951/test(public-safety): pin what a compacted value returns over its budget #4955. Base
mainat27f0fc93b. Related follow-up filed separately as [Bug]: a bytes field name is classified through its repr, so it can escape the public-safety rules #5069, which records a measured hole this PR deliberately does not pin as correct.Scope And Continuation
Completed scope and remaining work: tests only;
git diff --stat -- loopx/is empty. The set covers spelling equivalence for a family member, the suffix names that only the split classifies, the raw rule's camelCase reach, benign keys the boundary must not sweep up, and that the rejection names the caller's own key through nested mappings and lists.Not pinned, on purpose: a non-
strkey escapes the boundary rather than being classified, because the caller folds it withstr(key)— measured on currentmain,b"raw"normalizes tob_rawand is accepted while"raw"is rejected. I did not encode that as expected behaviour, since asserting it would make the hole harder to remove. test(public-safety): pin that a credential key is one key in any spelling #5070 records it with the reproduction and leaves the fix (reject non-strkeys, or decode them) to the owner.Slice boundary / successor: complete within this scope. The
SECRET_LIKE_SURFACE_PATTERN/LOCAL_PATH_SURFACE_PATTERNpre-normalisation rules are already covered bytests/control_plane/test_public_safety_path_shapes.py, and the compaction budget bytests/control_plane/test_public_safety_text_budget.py; this file adds the naming boundary and nothing else.Validation
c688d5fdbstaticpassedpython3 -m ruff checkon the new file;ruff format --diffreports no deviation for it;python3 -m mypywithout arguments as CI runs it —Success: no issues found in 23 source files(unchanged scope, since no production file moved);loopx check --scan-pathon the new file —public boundary scan clean: 1 files;git diff --checkclean.unitpassedtests/control_plane/test_public_safety_field_name_spelling.py— 21 passed: 7 spellings ofaccess_tokenfold to one key and one verdict, with the message asserted equal top.<caller's own spelling> is a credential-bearing field; 5 suffix-shaped camelCase names; the raw rule overraw,raw_body,rawOutput; 7 benign names that must stay acceptable; and the nestedp.outer.reviewToken/ listp[0].accessTokenpaths.regression_paritypassednormalize_public_safe_field_name/_is_credential_field/ the raw rule, each reverted before the next: removing the camelCase boundary fails 9; stopping the casefold fails 11; treating a space as part of the token fails 1; dropping the credential suffix tuple fails 6; dropping theraw_prefix fails 1. One mutation — removing the explicit.strip()— is not caught, and is an equivalent mutation:re.sub(r"[^A-Za-z0-9]+", "_")already folds leading and trailing spaces into underscores and.strip("_")then removes them, so the call cannot change any output. It is recorded rather than dropped. After restoring, this file plustest_public_safety_path_shapes.pyandtest_public_safety_text_budget.pyrun 52 passed, 0 failed.Coverage and gaps: the assertions sit at the one place the verdict is decided, and each mutation name corresponds to a distinct clause of the documented rule, so a future edit that weakens any clause is visible rather than silent. Values are covered by the sibling files named above and are not re-tested here.
Disclosed rather than smoothed: validation used a borrowed interpreter from an existing local virtualenv rather than a fresh
uv sync --extra test. This is a test-only change, so no full-suite before/after comparison is claimed; the three public-safety test files were run together as the affected neighbourhood.