Skip to content

Bind local chat to engine context limits - #80

Merged
andersalm merged 2 commits into
feat/0.7.1-modelsfrom
feat/0.7.1-model-context
Sep 30, 2026
Merged

andersalm merged 2 commits into
feat/0.7.1-modelsfrom
feat/0.7.1-model-context

Conversation

@andersalm

@andersalm andersalm commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Change

Assistant reads the selected local offer's per-slot context window and answer limit before it builds a chat turn. The model provider asks its owned llama engine to count the fully templated request, including special tokens, before it starts generation. It rejects a turn when the input and reserved answer exceed the slot. Counting and generation share the run deadline, and Stop can interrupt counting. Runtime admits only the exact token-count route through the local model broker.

The request carries the selected answer limit through the text input hash. Existing run journals keep their execution binding when the new offer metadata appears.

Proof and acceptance are tracked in #96.

Part of #96.

Publish per-slot limits to Assistant and request the selected answer reserve. Count the fully templated prompt through the owned llama engine before generation, with one deadline and cancellation path.

Verified: provider, model-contract, and runtime test suites; Assistant shell and cancellation tests; source gates and affected-crate Clippy.
@andersalm

Copy link
Copy Markdown
Contributor Author

@irzhywau ready for your review (security routine). Focus: elastos-runtime/src/provider/local_model_broker.rs and model-provider/src/execution.rs. Can any chat request reach the local engine without the Runtime-supplied context and answer limits, and are over-limit requests refused before generation? Part of #96 (LA-02).

@andersalm

Copy link
Copy Markdown
Contributor Author

@irzhywau this is today's priority review: Local AI is blocked on it (LA-02 closes when #80 merges, and LA-04 builds on it). Focus as in my earlier comment: can a chat request reach the local engine without the Runtime's limits, and are over-limit requests refused before generation?

@irzhywau

Copy link
Copy Markdown
Contributor

Security review — #80 Bind local chat to engine context limits

@andersalm — security review of head 0fa32099 against feat/0.7.1-models. Review only; no changes were made to the branch.

Verdict: approve from a security standpoint, with one Low fix recommended. No security invariant is violated: the broker stays an exact-route allowlist, inputs stay hash-bound, and nothing new crosses the capsule boundary.

Severity scale: Critical exploitable now with direct impact · High realistic path to code execution or integrity loss · Medium needs an extra condition · Low hardening / contract honesty · Info no action.


F1 — Low · max_output_tokens is accepted, hashed and then silently ignored for hosted offers

Where:

  • elastos/crates/elastos-model-contract/src/lib.rs:163 — validate_text_input_v2 now admits a third key max_output_tokens for every caller;
  • capsules/model-provider/src/adapters.rs:1299-1325 — validate_text_messages accepts v2 for AdapterConfig::OpenAiCompatibleText (hosted) as well as local;
  • capsules/model-provider/src/adapters.rs:2149-2158, 2184 — hosted request bodies use text_generation_max_tokens(offer) and never read the input value;
  • only requested_local_output_tokens (adapters.rs:1332) enforces it, for local llama only.

Threat. Contract dishonesty on a billed path. An App or peer sends max_output_tokens: 64 to a hosted, paid offer. Runtime accepts it and binds it into the input hash, so the journal and receipts claim a 64-token bound. The provider then generates up to the offer maximum. There is no privilege escalation, because the offer cap still applies, but a caller-requested spending or length bound is recorded as applied when it was not. Receipts and audit then disagree with what the provider actually did.

Fix (pick one):

  • Reject (simplest, fail-closed): in validate_text_messages, when input.get("max_output_tokens").is_some() and the adapter is not LocalLlamaCppText, return AdapterFault::context("model output request is not supported by this offer", …).
  • Honor it: in text_generation_request_body / the Responses body, use min(input.max_output_tokens, text_generation_max_tokens(offer)) for max_tokens / max_output_tokens. This needs the input value threaded through LocalTextBackend::OpenRouterDecisions / OpenAiResponses the same way requested_output_tokens is for local.

Also update docs/MODEL_PROVIDER.md (the new paragraph currently says "A text/v1 or text/v2 input can include max_output_tokens" without saying it is local-only).

Tests to add: a hosted fake offer with v2 input containing max_output_tokens, asserting either a context_rejected terminal or an upstream body carrying the reduced max_tokens.


F2 — Info · Cancel during token counting reports settlement_unknown

Where: adapters.rs:1821, 1828.

Stop before generation starts returns worker_settlement_unknown_result(), even though the provider knows no generation was sent. This is conservative and safe for local, unbilled use, but it makes the run look ambiguous in the journal. Consider a distinct "cancelled before dispatch" terminal. No security action required.


Checked and fine

  • Runtime broker allowlist (local_model_broker.rs:102-110, 178-186): exactly one new route, POST /v1/chat/completions/input_tokens. It keeps the same body bound (MAX_REQUEST_BODY_BYTES), requires a non-zero Content-Length, and still refuses chunked or duplicate lengths. Verified upstream that pinned llama.cpp b10516 registers this route in tools/server/server.cpp. Its sibling /v1/chat/completions/control remains denied.
  • Input binding: max_output_tokens is validated as an integer in 1..u32::MAX in the contract, then ≤ profile limit (local_output_token_limit, max 1024) at dispatch. The as u32 cast is safe after that bound.
  • Count response: read with read_bounded_json_response_async; a missing or zero input_tokens fails before any generation (test local_llama_missing_count_fails_before_generation).
  • Budget check: input + output >= slot rejects; slot = context_size / parallel, and config now requires ≥ 128 tokens per slot. --no-context-shift prevents the engine from silently dropping earlier turns, including the system prompt, when the window fills. Good for integrity of system instructions.
  • Deadline: counting and generation share the run deadline (the client is rebuilt from remaining_run_timeout).
  • Journal: stored context limits are range-checked on load (journal.rs:517+). Excluding context from the execution-binding comparison is sound because the underlying settings are still bound (test local_context_summary_uses_one_slot_and_rejects_unusable_profiles).
  • Assistant: a local offer without valid context limits fails closed (capabilitiesForOffer). A model switch during context preparation is refused (model_selection_changed). makeBudget clamps the reserve to the offer's maxOutputTokens, so the browser cannot request more than the provider allows.

@irzhywau irzhywau left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see comment above (non-blockling)

@andersalm

Copy link
Copy Markdown
Contributor Author

@irzhywau thank you, clear and fast. F1: we take your fail-closed option. max_output_tokens is rejected for offers that are not local, with your hosted-offer test and a "local only" note in docs/MODEL_PROVIDER.md. That fix goes into this PR before it merges. F2: agreed, no action now.

@andersalm

Copy link
Copy Markdown
Contributor Author

@irzhywau F1 is fixed in 07d525c46158c166417dea32e82afbd88276e9d5 using your rejection option. Hosted text inputs with max_output_tokens now end in context_rejected before dispatch. The regression test covers hosted Chat v2, Chat v1, and Responses v1 and checks that the fake upstream receives zero requests. It failed before the guard and passes with the fix.

docs/MODEL_PROVIDER.md now states that this field is local only. The provider suite passed with 233 unit tests and 6 process tests, and the basic source gate passed. The new candidate is pushed for CI. F2 stays informational, as agreed.

@andersalm
andersalm merged commit 0df365e into feat/0.7.1-models Sep 30, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants