From acf7955f872589cb4f0319fc5f537f020dd63f2d Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Wed, 22 Jul 2026 00:39:37 -0500 Subject: [PATCH] fix(git): fail closed on forbidden control chars, fix float exponent + '?' over-rejection (#865) Round-9 auditor blockers, both fixed at root cause in the PyYAML-absent line-parser fallback of get_gitea_token_for_login: 1. Control-character fail-open (dangerous): PyYAML 6.0.3's Reader rejects the WHOLE document (ReaderError) if a forbidden C0/DEL control byte {0x00-0x08, 0x0B, 0x0C, 0x0E-0x1F, 0x7F} appears ANYWHERE in the raw stream -- plain scalar, inside quotes, or a comment -- regardless of position, verified empirically against the real installed PyYAML 6.0.3. The fallback previously only guarded tabs and emitted the login token from such documents. Added a whole-document _FORBIDDEN_CONTROL scan on the raw text (before splitlines(), which itself splits on some of the same bytes) so the fallback fails closed identically to PyYAML. 2. Unsigned-exponent float over-rejection: PyYAML's implicit float resolver requires an EXPLICIT sign on the exponent; unsigned-exponent spellings (1.0e10, +1.0e10, -1.0e10, 1.0E10, .5e10, 4.e8) are PyYAML STRINGS, not floats. The fallback's _IMPLICIT_FLOAT pattern allowed an optional sign, misclassifying these as floats and dropping the token. Tightened the regex to match PyYAML's resolver exactly. Also folds in a residual over-rejection found via this round's wide differential fuzz (1767 cases, 0 fail-opens / 0 over-rejections after the fix): a plain scalar starting with "?" not followed by whitespace (e.g. "?x") is a valid PyYAML string, but the fallback's blanket-reject set treated every leading "?" as illegal. Narrowed to match the existing space-aware handling already used for "-" and ":". Adds regression fixtures to test-gitea-login-resolution.sh covering all three fixes under both PyYAML-present and forced-ImportError-absent runs. Co-Authored-By: Claude Opus 4.8 --- .../framework/tools/git/detect-platform.sh | 66 ++++++++-- .../tools/git/test-gitea-login-resolution.sh | 117 ++++++++++++++++++ 2 files changed, 171 insertions(+), 12 deletions(-) diff --git a/packages/mosaic/framework/tools/git/detect-platform.sh b/packages/mosaic/framework/tools/git/detect-platform.sh index e552743f..7d55ad10 100755 --- a/packages/mosaic/framework/tools/git/detect-platform.sh +++ b/packages/mosaic/framework/tools/git/detect-platform.sh @@ -640,9 +640,14 @@ _IMPLICIT_INT = re.compile( r"|[-+]?0x[0-9a-fA-F_]+" r"|[-+]?[1-9][0-9_]*(?::[0-5]?[0-9])+)$" ) +# NOTE: the exponent group requires an EXPLICIT sign ([eE][-+][0-9]+), matching +# PyYAML 6.0.3's resolver.py float regex exactly. An unsigned exponent (e.g. +# "1.0e10", ".5e10", "4.e8") is NOT matched by PyYAML's implicit float resolver +# -- PyYAML resolves those as plain strings -- so this pattern must not match +# them either, or the fallback over-rejects a token PyYAML would emit verbatim. _IMPLICIT_FLOAT = re.compile( - r"^(?:[-+]?(?:[0-9][0-9_]*)\.[0-9_]*(?:[eE][-+]?[0-9]+)?" - r"|\.[0-9][0-9_]*(?:[eE][-+]?[0-9]+)?" + r"^(?:[-+]?(?:[0-9][0-9_]*)\.[0-9_]*(?:[eE][-+][0-9]+)?" + r"|\.[0-9][0-9_]*(?:[eE][-+][0-9]+)?" r"|[-+]?[0-9][0-9_]*(?::[0-5]?[0-9])+\.[0-9_]*" r"|[-+]?\.(?:inf|Inf|INF)" r"|\.(?:nan|NaN|NAN))$" @@ -660,9 +665,10 @@ def _implicit_nonstring(text): # True when an UNQUOTED plain scalar would be resolved by PyYAML's SafeLoader # to a non-string type (null/bool/int/float/timestamp). Fuzzed against real # PyYAML 6.0.3: it never returns False where PyYAML types the scalar as a - # non-string (i.e. never fail-open), and is at worst MORE conservative on a - # couple of degenerate float spellings (e.g. "4.e8") that PyYAML keeps as a - # string -- the safe direction for this credential-selecting fallback. + # non-string (i.e. never fail-open), and (post exponent-sign fix) no longer + # over-rejects unsigned-exponent spellings like "4.e8" or "1.0e10" -- those + # match PyYAML's own implicit float resolver exactly (explicit sign only), + # so PyYAML keeps them as strings and this function now agrees. return bool( _IMPLICIT_NULL.match(text) or _IMPLICIT_BOOL.match(text) @@ -721,11 +727,10 @@ def _int_constructible(text): def _float_constructible(text): # Replicate PyYAML SafeConstructor.construct_yaml_float. Retained for the - # WHOLE-document invariant even though PyYAML's float-tagged set is always - # float()-constructible: the fallback's float RESOLVER pattern is a strict - # superset of PyYAML's (it also matches unsigned-exponent spellings PyYAML - # keeps as strings), and every such extra is likewise constructible, so this - # never fails closed where PyYAML would emit a token. + # WHOLE-document invariant: _IMPLICIT_FLOAT now matches PyYAML's resolver + # pattern exactly (explicit-sign exponent only), and every scalar it tags + # is float()-constructible, so this never fails closed where PyYAML would + # emit a token. value = text.replace("_", "").lower() if value[:1] in ("+", "-"): value = value[1:] @@ -804,6 +809,22 @@ def _constructible(text): return True +# PyYAML's Reader.check_printable scans the ENTIRE raw input stream (not just +# scalar contents, and NOT scoped by quoting) for bytes outside its printable +# set and raises ReaderError -- a whole-document reject -- the instant one is +# found, no matter where it sits (an unrelated field, a comment, inside or +# outside quotes). Empirically verified against real PyYAML 6.0.3: EVERY C0 +# control byte below plus DEL (0x7F) rejects in plain scalars, inside +# double-quoted scalars, and inside single-quoted scalars alike; only +# TAB(0x09), LF(0x0A), and CR(0x0D) are accepted among the C0 range (LF/CR are +# line separators, handled elsewhere; TAB has its own narrower, position-aware +# guard above since PyYAML accepts it in some contexts). This set therefore +# excludes 0x09/0x0A/0x0D and matches the forbidden C0 set {0x00-0x08, 0x0B, +# 0x0C, 0x0E-0x1F} plus DEL (0x7F). +_FORBIDDEN_CONTROL = re.compile( + "[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]" +) + # Sentinel: "outside the supported subset -> fail closed". Distinct from a # genuine null (None), which is a valid resolved value. _FAIL = object() @@ -865,8 +886,15 @@ def _scalar(raw): value = value.strip(" ") # spaces only; a tab must survive to fail closed if not value: return None - if value[0] in _FLOW or value[0] in ("|", ">", "?", "@", "`", '"', "'", "%", ","): + if value[0] in _FLOW or value[0] in ("|", ">", "@", "`", '"', "'", "%", ","): return _FAIL # flow / block-scalar / reserved / directive / anchor / quote + # NOTE: "?" is deliberately NOT in the blanket-reject set above. Empirically + # verified against real PyYAML 6.0.3: "?x" (indicator immediately followed by + # a non-space) is a plain scalar string "?x" like "-x" and ":x" are -- only + # "?" alone or "? " (followed by space/EOL) opens a complex mapping key and + # is illegal in a value position. Blanket-rejecting every leading "?" would + # over-reject a token PyYAML accepts verbatim; the narrower check below + # (mirroring "-" and ":") handles exactly the illegal bare-indicator forms. if value[0] in ("-", "?", ":") and (len(value) == 1 or value[1] in (" ", "\t")): # A bare block indicator, not a plain scalar: '-'/'- ' opens a sequence # entry, '?'/'? ' a complex mapping key, ':'/': ' a mapping value -- all @@ -1151,7 +1179,21 @@ def _token_via_lines(): # whose `name` equals the request -> host/port-bound token), so the fallback # can only ever be MORE conservative than PyYAML, never less. with open(config_path, encoding="utf-8") as handle: - lines = handle.read().splitlines() + raw_text = handle.read() + + # Whole-document, position-independent reject: PyYAML's Reader rejects the + # ENTIRE document (ReaderError) if a forbidden C0/DEL control byte appears + # ANYWHERE in the raw stream -- in a plain scalar, inside single- or + # double-quoted scalars, in a comment, or in a field this recognizer never + # even looks at. Checking the raw text (before splitlines(), which itself + # splits on some of these same control bytes -- e.g. \x0b, \x0c, \x1c-\x1e + # -- and would otherwise obscure them) mirrors that exactly: fail closed + # for the whole document rather than risk emitting a token PyYAML would + # have refused. + if _FORBIDDEN_CONTROL.search(raw_text): + return None + + lines = raw_text.splitlines() config = _safe_parse(lines) if config is _FAIL: diff --git a/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh b/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh index c6d4bdb6..1445f924 100755 --- a/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh +++ b/packages/mosaic/framework/tools/git/test-gitea-login-resolution.sh @@ -776,4 +776,121 @@ write_fixture "logins: " assert_token "quoted token with inner tab resolves verbatim" "T${TAB}OK" primary git.example +# 18. CONTROL-CHARACTER FAIL-CLOSE (#865 round-9 blocker 1): PyYAML's Reader +# scans the ENTIRE raw document stream (not merely scalar contents, and NOT +# scoped by quoting) for bytes outside its printable set and raises +# ReaderError -- a WHOLE-DOCUMENT reject -- the instant one is found, +# regardless of where it sits: an unrelated field's plain scalar, inside a +# double- or single-quoted scalar, or a comment. Verified empirically against +# real installed PyYAML 6.0.3 (see detect-platform.sh's _FORBIDDEN_CONTROL +# comment): every C0 control byte {0x00-0x08, 0x0B, 0x0C, 0x0E-0x1F} plus DEL +# (0x7F) rejects in ALL THREE contexts (plain / double-quoted / single-quoted); +# only TAB(0x09), LF(0x0A), CR(0x0D) are accepted among the low byte range +# (TAB has its own narrower, position-aware coverage in section 17 above; LF/CR +# are line separators). A prior fallback ONLY guarded tabs and emitted the +# login token from documents PyYAML rejects over an UNRELATED field's control +# byte -- a dangerous fail-open (credential emission from a document PyYAML +# refuses). write_control_char_fixture writes the raw byte directly via +# printf's octal escape (never through a bash string/variable, which cannot +# hold an embedded NUL) so 0x00 is exercised faithfully alongside the rest. +write_control_char_fixture() { + local octal="$1" quote="${2:-}" + { + if [[ -n "$quote" ]]; then + printf 'bad: %sx' "$quote" + # shellcheck disable=SC2059 # deliberate: $octal supplies printf's + # own \NNN octal escape so the raw control byte reaches the file + # directly, never passing through a bash string (which truncates + # at an embedded NUL and so cannot represent byte 0x00 otherwise). + printf "\\${octal}" + printf 'y%s\n' "$quote" + else + printf 'bad: x' + # shellcheck disable=SC2059 # deliberate: $octal supplies printf's + # own \NNN octal escape so the raw control byte reaches the file + # directly, never passing through a bash string (which truncates + # at an embedded NUL and so cannot represent byte 0x00 otherwise). + printf "\\${octal}" + printf 'y\n' + fi + printf 'logins:\n - name: primary\n url: https://git.example\n token: TOK_PLAIN\n' + } > "$FIXTURE_XDG/tea/config.yml" +} + +# Plain (unquoted) unrelated-field placement: the full empirically-confirmed +# forbidden C0/DEL set. +for octal in 000 001 002 003 004 005 006 007 010 013 014 \ + 016 017 020 021 022 023 024 025 026 027 \ + 030 031 032 033 034 035 036 037 177; do + write_control_char_fixture "$octal" + assert_both_fail_closed "control byte \\$octal in unrelated plain field fails closed" primary git.example +done + +# Inside-quote variants (double and single) for the six bytes called out +# explicitly in the round-9 blocker report: 0x00,0x01,0x07,0x0e,0x1f,0x7f. +for octal in 000 001 007 016 037 177; do + write_control_char_fixture "$octal" '"' + assert_both_fail_closed "control byte \\$octal inside double-quoted unrelated field fails closed" primary git.example + write_control_char_fixture "$octal" "'" + assert_both_fail_closed "control byte \\$octal inside single-quoted unrelated field fails closed" primary git.example +done + +# Same forbidden byte inside a comment line -- PyYAML's Reader check is +# stream-wide, so it rejects here too, not merely inside live scalar content. +{ + printf '# note' + printf '\007' + printf 'here\nlogins:\n - name: primary\n url: https://git.example\n token: TOK_PLAIN\n' +} > "$FIXTURE_XDG/tea/config.yml" +assert_both_fail_closed "control byte in a comment line fails closed" primary git.example + +# NOT over-broad: TAB/LF/CR remain accepted where PyYAML already accepts them +# (covered by section 17's tab fixtures and the ordinary newline-delimited +# fixtures used throughout this file), so no additional assertion is needed +# here beyond confirming the forbidden-control guard does not fire on them. + +# 19. UNSIGNED-EXPONENT FLOAT OVER-REJECTION (#865 round-9 blocker 2): PyYAML +# 6.0.3's implicit float resolver requires an EXPLICIT SIGN on the exponent +# ([eE][-+][0-9]+); an unsigned exponent is NOT matched, so PyYAML resolves +# the scalar as a plain STRING, not a float. A prior fallback's float +# recognizer accepted an OPTIONAL sign ([eE][-+]?[0-9]+), over-matching these +# spellings as floats and dropping the token PyYAML would emit verbatim +# (over-rejection). Verified empirically against real PyYAML 6.0.3. +for form in '1.0e10' '+1.0e10' '-1.0e10' '1.0E10' '.5e10' '4.e8'; do + write_fixture "logins: + - name: primary + url: https://git.example + token: ${form} +" + assert_token "unsigned-exponent form '$form' is a PyYAML string, token resolves" "$form" primary git.example +done + +# Parity guard: a genuine SIGNED-exponent float is still typed as a non-string +# float by PyYAML and must still fail closed (not regress into over-acceptance). +write_fixture 'logins: + - name: primary + url: https://git.example + token: 1.0e+10 +' +assert_token "signed-exponent genuine float still fails closed" "" primary git.example +write_fixture 'logins: + - name: primary + url: https://git.example + token: 1.0e-10 +' +assert_token "signed-exponent (negative) genuine float still fails closed" "" primary git.example + +# 20. RESIDUAL OVER-REJECTION found via round-9 differential fuzzing (folded into +# this round, not split off): a plain scalar starting with "?" NOT followed by +# whitespace (e.g. "?x") is a valid PyYAML string -- only a bare "?" or "? " +# (question mark followed by space/EOL) opens a complex mapping key and is +# illegal in a value position. A prior blanket-reject set treated EVERY +# leading "?" as illegal, over-rejecting a token PyYAML accepts verbatim. +write_fixture 'logins: + - name: primary + url: https://git.example + token: ?x +' +assert_token "question-mark-not-space is a plain string, token resolves" "?x" primary git.example + echo "Gitea login resolution regression harness passed"