fix(unified): strict ingest contract, read by shape, bounded recall, caps, hook budget (PRO-2193) - #14
Conversation
… bound recall, respect caps (PRO-2193) Found by checking #13 against staging (unified switch on, the strict ingest decoder from hydradb-application#1653, the caps from #1657): - A turn is exactly {role, content}: the per-turn `name` was a 400, which in the Stop hook also lost the turn's state. The speaker is the item's `user_name` on text and conversation alike; title and instructions are clipped to the server's limits. - The 202 names the context `id`; context ids were always empty. - Answers are read by shape: a v2 body from a unified database (unified surface off, older server) is read as v2 instead of making recall "unavailable", and a body without forceful_relations is unified. - The injected llm_prompt was unbounded; Claude Code moves hook context past ~10k characters to a file and shows a preview. It is now fitted to maxContextChars by shortening result bodies located by the recall's own chunk text (Markdown bodies are never parsed as structure), keeping every heading, id and citation, with a last-resort cut. The query skill output is bounded the same way. - Unified writes go out within the caps (100 items, 8 MiB text per request; an item over 1 MiB is refused by name, the rest still sent). Workspace sync cuts files to 250k-character pieces on a unified database and routes a file too large to go whole through the chunked path; the session transcript keeps its latest turns within 1 MiB. - The layout probe has a 3s timeout and its answer is cached on disk for 10 minutes, so a hook no longer spends its 20s budget on it; a failed probe is logged and read as unknown, and only an unknown layout is retried as unified. - Resilience (both layouts): a failed capture no longer loses the Stop hook's state, and a refused sync batch no longer aborts the others; its files stay unsynced so the next sync retries them. Tests: conformance/unified-fixes.mjs (7 groups; the behavioural ones fail on #13), existing tests updated where they pinned the old contract. Version 1.2.0 in plugin.json, marketplace.json and package.json. Signed-off-by: Harsh Ranjan <harsh.ranjan@hydradb.com>
…tool-output cut (PRO-2193) A unified recall carries the same text twice (llm_prompt and chunks[]). Live on staging the --json payload was 43k characters with the earlier caps, over the ~30k the host shows of a tool result. llm_prompt is now fitted to 12k and each chunk body to 800 characters (flagged with its full length): 23k on the same query. Signed-off-by: Harsh Ranjan <harsh.ranjan@hydradb.com>
|
OpenHack SummarySecurity review of fix(unified): strict ingest contract, read by shape, bounded recall, caps, hook budget (PRO-2193). 13 changed files; 1 finding at or above the low reporting threshold. Confidence Score: 4/5Review the findings below before merging. Security merge-readiness rubric: 1 = critical, 2 = high, 3 = medium, 4 = low, 5 = no reportable findings. This score reflects scan findings, not a guarantee of correctness or complete coverage. Files Needing Attention: scripts/lib/context-format.mjs Important Files Changed
Prompt To Fix With AIReview the findings for https://github.com/hydra-db/hydradb-claude-code/pull/14 at commit 9475282cc07048d6e2bcbfdb12be0474c2a917fb. Verify each finding against the current code before fixing it. Preserve unrelated changes and run focused regression tests. ### Issue 1: [P4] Server-controlled unified recall processing can cause algorithmic denial of service Vulnerability type: Denial of Service scripts/lib/context-format.mjs:291 This PR changes unified recall handling from largely pass-through rendering to synchronous fitting and bounding of server-provided content. fitUnifiedPrompt locates every occurrence of each chunk content or enrichment in the full llm_prompt and checks overlap against all previously located intervals. For oversized prompts, its fallback line-shortening loop repeatedly evaluates the full joined text while shortening candidates. Separately, fitRecallPayload measures the complete JSON payload with JSON.stringify on each iteration while removing graph and forceful-relation entries one at a time. Because response size and array counts are not bounded before these operations, a hostile or compromised service response, or attacker-controlled corpus content reflected by the service, can cause excessive CPU use and stall recall or hook execution. The old implementation returned the unified prompt and emitted the recall payload without these new processing loops. Recommendation: Bound server-controlled input before any expensive processing: cap llm_prompt length, chunk/enrichment lengths, body count, occurrence count, graph paths, and forceful relations. Replace repeated full-payload JSON.stringify/pop loops with a single size calculation or geometric/batched removal and retain truncation metadata. Replace repeated lines.join calls with a running length total and join once. Use sorted or indexed interval handling for body occurrences, and enforce a global work budget that falls back directly to a bounded prefix cut. Add regression tests for large prompts, repeated bodies, and large graph arrays that assert bounded runtime and output size. Last reviewed commit: 9475282 · View review on OpenHack
|
…layout, bound --json (PRO-2193) - A turn whose capture fails is queued in the session state (max 20) and retried on the next Stop under the same source id. - Workspace sync waits for a patient layout probe (full request timeout) before choosing a chunk size, so a slow first probe no longer sends unbounded chunks to a unified database. - fitUnifiedPrompt finds every copy of a body, and before the last-resort cut it shortens long non-structural lines, so later headings, ids and citation labels survive. - The query skill's --json payload is reduced step by step until it fits QUERY_JSON_CHARS, keeping ids and scores. - The layout cache is written to a temp file and renamed into place, so parallel hooks never read a half-written file. Signed-off-by: Harsh Ranjan <harsh.ranjan@hydradb.com>
…k budget (PRO-2193) The Stop hook now queues and writes the current turn to state before any network call, sends it first, and retries older queued turns only within 8s of the hook's 20s budget and only while the service is answering: the first failure ends the retries. A long outage costs one failed request per Stop instead of one per queued turn, and a hook the host kills mid-request no longer loses the turn or retry progress. Signed-off-by: Harsh Ranjan <harsh.ranjan@hydradb.com>
Two overlapping Stop hooks each wrote their own copy of the queue, and the later write replaced the earlier one, dropping a turn that had already been marked as seen. Queued turns are now merged by source id on every state write; an entry leaves the queue only when the writer that saved (or dropped) it names it in removedTurnCaptures. Signed-off-by: Harsh Ranjan <harsh.ranjan@hydradb.com>
| // marked. Headings, ids, labels and the related-facts section are never | ||
| // touched. If the prompt is still over budget, it is cut at a line with a | ||
| // note, so the bound always holds. A prompt that fits is returned untouched. | ||
| export function fitUnifiedPrompt(result, maxChars) { |
There was a problem hiding this comment.
Server-controlled unified recall processing can cause algorithmic denial of service
Vulnerability type: Denial of Service
This PR changes unified recall handling from largely pass-through rendering to synchronous fitting and bounding of server-provided content. fitUnifiedPrompt locates every occurrence of each chunk content or enrichment in the full llm_prompt and checks overlap against all previously located intervals. For oversized prompts, its fallback line-shortening loop repeatedly evaluates the full joined text while shortening candidates. Separately, fitRecallPayload measures the complete JSON payload with JSON.stringify on each iteration while removing graph and forceful-relation entries one at a time. Because response size and array counts are not bounded before these operations, a hostile or compromised service response, or attacker-controlled corpus content reflected by the service, can cause excessive CPU use and stall recall or hook execution. The old implementation returned the unified prompt and emitted the recall payload without these new processing loops.
Location: scripts/lib/context-format.mjs:291
Recommendation:
Bound server-controlled input before any expensive processing: cap llm_prompt length, chunk/enrichment lengths, body count, occurrence count, graph paths, and forceful relations. Replace repeated full-payload JSON.stringify/pop loops with a single size calculation or geometric/batched removal and retain truncation metadata. Replace repeated lines.join calls with a running length total and join once. Use sorted or indexed interval handling for body occurrences, and enforce a global work budget that falls back directly to a bounded prefix cut. Add regression tests for large prompts, repeated bodies, and large graph arrays that assert bounded runtime and output size.
Prompt To Fix With AI
Review the findings for https://github.com/hydra-db/hydradb-claude-code/pull/14 at commit 9475282cc07048d6e2bcbfdb12be0474c2a917fb. Verify each finding against the current code before fixing it. Preserve unrelated changes and run focused regression tests. ### Issue 1: [P4] Server-controlled unified recall processing can cause algorithmic denial of service Vulnerability type: Denial of Service scripts/lib/context-format.mjs:291 This PR changes unified recall handling from largely pass-through rendering to synchronous fitting and bounding of server-provided content. fitUnifiedPrompt locates every occurrence of each chunk content or enrichment in the full llm_prompt and checks overlap against all previously located intervals. For oversized prompts, its fallback line-shortening loop repeatedly evaluates the full joined text while shortening candidates. Separately, fitRecallPayload measures the complete JSON payload with JSON.stringify on each iteration while removing graph and forceful-relation entries one at a time. Because response size and array counts are not bounded before these operations, a hostile or compromised service response, or attacker-controlled corpus content reflected by the service, can cause excessive CPU use and stall recall or hook execution. The old implementation returned the unified prompt and emitted the recall payload without these new processing loops. Recommendation: Bound server-controlled input before any expensive processing: cap llm_prompt length, chunk/enrichment lengths, body count, occurrence count, graph paths, and forceful relations. Replace repeated full-payload JSON.stringify/pop loops with a single size calculation or geometric/batched removal and retain truncation metadata. Replace repeated lines.join calls with a running length total and join once. Use sorted or indexed interval handling for body occurrences, and enforce a global work budget that falls back directly to a bounded prefix cut. Add regression tests for large prompts, repeated bodies, and large graph arrays that assert bounded runtime and output size.
TIP: Reply
@openhack-agentor@openhack-agent fix thisto fix this finding. To ask a question, mention@openhack-agentfollowed by your question.
Stacked on #13 (
soham/pro-1618-unified-contract). It fixes what #13 hits against staging today, where the unified switch is on, the strict ingest decoder from hydra-db/hydradb-application#1653 is live, and the caps from #1657 apply. It also bumps the plugin to 1.2.0.Fixes
{role, content}. The per-turnnamewas a 400, and in the Stop hook that also lost the turn's state.user_name, for text and conversation alike.id;source_idis kept as a fallback. Before,contextIdswas always empty.forceful_relationsis unified, with no forceful relations.additionalContextpast about 10k characters to a file and shows the model only a preview. The unifiedllm_promptwas injected whole, 27k characters on a real staging query.maxContextChars(7000). Result bodies are located by the recall's own chunk text and shortened in place, so Markdown inside a body is never parsed as structure.[n]/[Rn]/[Pn]label is kept, and a last-resort cut guarantees the bound.queryskill's unified--jsonoutput stays under the host's tool-output cut: 12k for the prompt, 800 per chunk, flagged.layout-cache.json, 10 min, keyed by server, hashed key and database). Hooks no longer pay up to 15 s of their 20 s budget on it.debug.logand read as unknown, and only an unknown layout is retried as unified.Split databases send the same requests as 1.1.0. The resilience changes are the only behaviour change there, and they only apply when a write fails.
Testing
Automated
npm run check: all existing suites (16 conformance vectors, 29 wire tests, 6 goldens) plus a newconformance/unified-fixes.mjswith 7 groups.namerefused, empty context ids, v2 body refused).name,source_id, the unbounded block, the sync aborting) are updated.Live, on staging (the real hook commands fed Claude Code's hook input, split and unified databases):
userNamesetingest --notecontext idquery --jsonNotes
main, so it will not run on this stacked PR until Unified databases (PRO-1618): layout-aware recall and ingest, items[] body #11/Unified databases (PRO-1618): adopt the client contract (no type, context[] body, four-key query, llm_prompt verbatim) #13 land.hydra-db/hydradb-codexis a copy of this plugin at 1.1.0; these changes should be ported there next.