fix: keep unusable DOCX numeric references as literal text

[mldangelo]

Fixes #40.

Summary

decodeXml in src/knowledge-base.ts passed the parsed value of a numeric character reference straight to String.fromCodePoint with no magnitude bound. The regex admits #\d+ and #x[\da-f]+ with no upper limit, so � and � both reached it and threw RangeError.

That propagated out of extractDocx as Cannot extract text from knowledge base DOCX: <path> — which reads like a corrupt or unreadable file and sends you looking in the wrong place, with the real RangeError visible only via the error's cause — and failed the entire prepareKnowledgeBase call. One stray reference in one document aborted the scan before Codex started, taking every other knowledge-base file with it.

What this changes

A reference that cannot name a Unicode scalar value is left as literal text, exactly matching the entities[name.toLowerCase()] ?? entity fallback for unrecognized named entities on the line above. This is the direction suggested in the issue.

I also excluded surrogates (U+D800–U+DFFF), which the issue's suggested snippet does not cover. They do not throw — String.fromCodePoint(0xD800) happily returns a lone surrogate — so this is a separate, quieter bug: XML forbids surrogates in the Char production, and the lone surrogate would then be written out by writeFile(..., "utf8") as U+FFFD, silently corrupting the extracted text rather than failing. Same root cause, same one-line guard, so it seemed wrong to leave it.

function isUnicodeScalarValue(codePoint: number): boolean {
  return (
    Number.isInteger(codePoint) &&
    codePoint >= 0 &&
    codePoint <= MAX_CODE_POINT &&
    !(codePoint >= 0xd800 && codePoint <= 0xdfff)
  );
}

Number.isInteger covers the NaN case; huge decimals like � parse to 1e20, which is an integer by float semantics but is caught by the upper bound.

Deliberately out of scope

The broader policy question. The issue's Impact section objects that "a single stray entity anywhere in one document blocks the entire scan." With the decode fixed, that no longer happens for this cause — the document extracts normally. But prepareKnowledgeBase still aborts wholesale for a genuinely unextractable document, and I did not change that, because failing loudly may well be the right behavior for a security knowledge base: silently proceeding with a document missing is arguably worse than refusing to start. If you want skip-and-warn semantics instead, that is a deliberate product change and I am happy to do it separately.

The full XML Char production. � and other C0 controls still decode, so a NUL can reach the extracted text. XML forbids those too (Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF]), and implementing the production in full would be a few more lines. I left it out because it is neither a crash nor a silent corruption, and it is not what the issue reports. Flagging it as a possible follow-up.

Testing / QA instructions

Baseline before this branch: 470 pass / 6 skip / 0 fail. After: 472 pass / 6 skip / 0 fail (two added tests).

cd sdk/typescript          # run pnpm from here, not the repo root
CI=true pnpm install --frozen-lockfile
CI=true pnpm run types
CI=true pnpm run test
CI=true pnpm run format
CI=true pnpm run build

Confirm the tests reproduce the issue

Both added tests must fail against main's source. Revert only the source, keeping the tests (plain git stash will not work — the tests are in a different file, but reverting both defeats the check):

cd sdk/typescript
cp src/knowledge-base.ts /tmp/fixed.ts
git checkout main -- src/knowledge-base.ts
CI=true bun test --timeout 30000 ./tests-ts/knowledge-base.test.ts
#   expect 7 pass / 2 fail, with:
#   RangeError: Arguments contain a value that is out of range of code points
cp /tmp/fixed.ts src/knowledge-base.ts
CI=true bun test --timeout 30000 ./tests-ts/knowledge-base.test.ts   # expect 9 pass / 0 fail

New coverage

Both tests build a real .docx through the existing docx() helper (fflate zipSync of word/document.xml) and assert on the extracted text, so they exercise the whole prepareKnowledgeBase → extractDocx → decodeXml path rather than the decoder in isolation.

1. keeps DOCX numeric references that cannot name a code point as literal text — seven cases, covering both the regression and the no-regression side:

   Input	Expected
   �	left literal
   �	left literal
   �	left literal
   � (lone surrogate)	left literal
   A	decodes to A
   😀	decodes to 😀
   	decodes (boundary must still work)

   The last three matter as much as the first four — the bound must not clip valid astral characters or the max code point.

2. keeps one unusable reference from failing the other knowledge-base documents — a directory holding one good .md and one .docx with an out-of-range reference. Asserts both documents are extracted. This is the user-visible impact from the issue: on main the .md is lost too.

Manual reproduction from the issue

cd sdk/typescript && bun -e '
import { zipSync, strToU8 } from "fflate";
import { writeFile, mkdtemp } from "node:fs/promises";
import { tmpdir } from "node:os"; import { join } from "node:path";
import { prepareKnowledgeBase } from "./src/knowledge-base.js";
const dir = await mkdtemp(join(tmpdir(), "kb-"));
const xml = `<?xml version="1.0"?><w:document xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"><w:body><w:p><w:r><w:t>Boundary &#x110000; case.</w:t></w:r></w:p></w:body></w:document>`;
await writeFile(join(dir, "threat-model.docx"), zipSync({ "word/document.xml": strToU8(xml) }));
const kb = await prepareKnowledgeBase([join(dir, "threat-model.docx")]);
console.log("prepared:", kb.path); await kb.cleanup();'

Expect prepared: ... on this branch; on main it throws Cannot extract text from knowledge base DOCX.

Note for maintainers

PRs #117 and #95 are competing rewrites of this same file (both add MAX_DOCUMENTS/MAX_DIRECTORY_DEPTH/size caps). Neither fixes this — I checked, they only rename the decodeXml(...) call site to const text = decodeXml(...). This PR touches only decodeXml and one new constant, so it should rebase cleanly onto whichever of those you land, but merge order matters.

[mldangelo-oai]

@codex review Please review exact head be21573, focusing on DOCX numeric entity bounds, Unicode scalar validation, invalid references, preserving unrelated documents, and the current-main merge.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 👍

Reviewed commit: be2157343c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Security review completed. No security issues were found in this pull request.

Reviewed commit: be2157343c

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

• You comment "@codex security review"
• A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.