Context
Surfaced during the Fable adversarial review of LeanerCloud/cloud-commitments-cli#1515 (Azure RI-exchange execute). Pre-existing, affects the AWS execute path identically -- NOT introduced by LeanerCloud/cloud-commitments-cli#1515.
Issue
matchPurchaseAmountConstraint (internal/auth/service_group.go ~:269) gates on permMax > 0: a permission whose stored MaxPurchaseAmount is negative is treated as no amount constraint (allow), rather than deny-all. If admin input validation does not reject negative MaxPurchaseAmount values at write time, a negative value silently disables the spend cap on that permission.
Fix
Treat permMax < 0 as deny-all in the matcher, rather than as 'no constraint'. One condition, fails closed, and it covers permission rows that are already stored negative, which a write-time validation cannot.
Rejecting negatives at the write boundary is worth adding as well if it is cheap, but it is not the fix on its own: it leaves existing malformed rows fail-open.
Leave permMax == 0 alone. Zero is the unset zero-value here, and folding it into the same condition would turn every permission with no configured cap into deny-all.
Notes
- Non-exploitable via the API today (JSON request bodies can't inject a negative stored permission cap; this is about stored permission data), but it's a real fail-open on malformed/negative admin config.
- There is a cosmetic asymmetry on the currency compare (EqualFold on the constraint path vs case-sensitive in the money guardrail; both fail closed). Left alone deliberately: harmonising it by uppercase-normalising
body.Currency adds a normaliser on a spend-cap path, whose whole purpose is to make two different strings compare equal. Neither compare is wrong today. If the asymmetry is worth removing, make the constraint path case-sensitive to match the guardrail, in its own change.
Remedy simplified (2026-08-03): the "and/or" became one fail-closed condition in the matcher, with the write-boundary check noted as insufficient alone; the currency normaliser was dropped from this issue. Bundling a normaliser onto a money path into an unrelated fail-open fix is how the repo's substring-for-equality defects have arrived before, and PR LeanerCloud/cloud-commitments-cli#1515 already settled that a non-USD money path must fail closed rather than be normalised into comparability.
Context
Surfaced during the Fable adversarial review of LeanerCloud/cloud-commitments-cli#1515 (Azure RI-exchange execute). Pre-existing, affects the AWS execute path identically -- NOT introduced by LeanerCloud/cloud-commitments-cli#1515.
Issue
matchPurchaseAmountConstraint(internal/auth/service_group.go ~:269) gates onpermMax > 0: a permission whose storedMaxPurchaseAmountis negative is treated as no amount constraint (allow), rather than deny-all. If admin input validation does not reject negative MaxPurchaseAmount values at write time, a negative value silently disables the spend cap on that permission.Fix
Treat
permMax < 0as deny-all in the matcher, rather than as 'no constraint'. One condition, fails closed, and it covers permission rows that are already stored negative, which a write-time validation cannot.Rejecting negatives at the write boundary is worth adding as well if it is cheap, but it is not the fix on its own: it leaves existing malformed rows fail-open.
Leave
permMax == 0alone. Zero is the unset zero-value here, and folding it into the same condition would turn every permission with no configured cap into deny-all.Notes
body.Currencyadds a normaliser on a spend-cap path, whose whole purpose is to make two different strings compare equal. Neither compare is wrong today. If the asymmetry is worth removing, make the constraint path case-sensitive to match the guardrail, in its own change.Remedy simplified (2026-08-03): the "and/or" became one fail-closed condition in the matcher, with the write-boundary check noted as insufficient alone; the currency normaliser was dropped from this issue. Bundling a normaliser onto a money path into an unrelated fail-open fix is how the repo's substring-for-equality defects have arrived before, and PR LeanerCloud/cloud-commitments-cli#1515 already settled that a non-USD money path must fail closed rather than be normalised into comparability.