Skip to content

Controller schemas cannot express an integer narrower than 64 bits, so out-of-range values pass validation #6137

Description

@M3gA-Mind

Summary

core::TypeSchema has no unsigned type narrower than U64 and no numeric bounds, so any controller field backed by a u32 / u16 / u8 / NonZero* is declared as U64. A value inside u64 but outside the real type passes validate_params and is then refused by the handler's deserialization — a schema-driven caller is told the value is valid right up until the call fails.

Problem

src/core/mod.rs declares the numeric shapes as I64, U64 and F64, and check_type (src/core/all.rs:1478-1480) tests only value.is_u64() / is_i64() / is_f64(). There is no narrower variant and no min / max, so a schema cannot say "0..=4294967295".

Steps to reproduce (found reviewing #6117, where todos.replace's card schema was made truthful):

  1. TaskBoardCard::order is a u32.
  2. Its schema entry is TypeSchema::U64 — the closest declaration available.
  3. Call openhuman.todos_replace with a card whose order is 4294967296.
  4. validate_params accepts it (is_u64() is true).
  5. The handler's serde_json::from_value::<TaskBoardCard> rejects it, and the caller gets a generic invalid params for a value the catalog advertised as in-type.

Expected: the schema either rejects the value with a bound-specific message, or does not advertise a range it cannot enforce.

Impact. Two things consume this catalog and both are misled:

  • GET /schema and the generated TS types are what a frontend or an external RPC caller builds requests from.
  • The tool-schema surface is what a model is shown, so a declared range is read as a promise.

The failure is quiet — it looks like a caller bug rather than a schema bug — and the wrong-looking rejection happens after the round trip, not before it.

Scope. This is not specific to order. src/openhuman/ carries 208 TypeSchema::U64 declarations (213 across src/), plus 27 I64 and 36 F64, and every one whose backing field is narrower than 64 bits has the same gap. Densest: memory/sources/schemas_part_01.rs (23), memory/schema/schema_schema_part_01.rs (23), agent/learning/schemas_part_01.rs (13), memory/schemas/documents.rs (10). Nobody has audited which of those are genuinely u64.

Solution (optional)

Not proposed as decided — the shape is the decision to make here. Two candidates:

  1. A bounded numeric variant — e.g. U64 { min: Option<u64>, max: Option<u64> } or a separate Uint { bits } — with check_type doing the range test and naming the bound in the error.
  2. Keep the variants and add an optional constraint on FieldSchema, so the numeric types stay a closed set and any type can carry a bound.

Either way the work is the same shape, and the cost is in the consumers rather than the enum: check_type and the validate_params error wording, the /schema serialization (TypeSchema is Serialize and its JSON shape is a public contract — a struct-like variant changes it), the checked-in app/schema.json, the hand-written TS types, and the CLI/RPC smoke-test generation.

Worth deciding first, because it changes the size of the job:

  • Does this need a migration of the 208 sites, or only new ones? Declaring U64 where the field is a u32 stays sound — it over-accepts, it never under-accepts — so an incremental migration is safe and a big-bang rewrite is not required for correctness.
  • Is the /schema wire shape allowed to change? If not, the bound has to ride somewhere additive.

Prose is the current workaround, and it is what #6117 shipped for this one field: "Sort position, 0..=4294967295 (u32) … a value above the u32 ceiling is refused by the handler rather than by schema validation." That keeps a human caller honest and does nothing for a machine one.

Acceptance criteria

  • A bound is expressible — a controller field backed by a u32 can declare its real range, and validate_params rejects a value outside it before the handler is reached.
  • The error names the bound — the rejection says which limit was exceeded, in the same vocabulary as the existing invalid type for param '<name>' in <ns>.<fn> messages.
  • The /schema contract is settled — either the JSON shape is unchanged, or app/schema.json and the TS types are regenerated in the same change.
  • todos.replace's order is migrated as the first consumer, and the prose caveat added in fix(todos): declare the real card shape on todos_replace's schema #6117 is removed with it.
  • Regression safety — a test proving an out-of-range value is refused by validation rather than by handler deserialization.
  • Diff coverage ≥ 80%.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority: p1Next. Wrong behaviour a user will hit, or a security weakness behind a condition.

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions