Skip to content

fix: bound knowledge-base discovery and document extraction - #117

Closed
mldangelo-oai wants to merge 22 commits into
mainfrom
mdangelo/codex/bounded-knowledge-base-ingestion
Closed

fix: bound knowledge-base discovery and document extraction#117
mldangelo-oai wants to merge 22 commits into
mainfrom
mdangelo/codex/bounded-knowledge-base-ingestion

Conversation

@mldangelo-oai

Copy link
Copy Markdown
Collaborator

Summary

  • Adopt the original knowledge-base hardening contribution from PR fix: bound knowledge-base ingestion #95 while preserving the original substantive commit author, kartikshukla17, and acknowledging GautamSharma99 for submitting and maintaining the source PR.
  • Bound discovery depth, entries, document count, per-document and aggregate input, extracted text, PDF pages, and cancellation.
  • Preserve secure source handling, private temporary outputs, and existing public SDK behavior.

Verification

  • Full current-main SDK suite: 474 passed, six expected platform/integration skips, zero failures.
  • Focused knowledge-base and orchestration coverage: 78 passed.
  • TypeScript and formatting checks passed.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review exact head c95cc9f, preserving the original contributor authorship while auditing discovery bounds, cancellation, PDF/DOCX extraction limits, symlink safety, and 474 passing current-main SDK tests.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c95cc9f92c

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review exact pushed head 3000c1f, including contributor authorship, incremental PDF text-stream bounds, parent-directory containment checks, nonblocking FIFO protection, deterministic race regressions, and the latest main merge.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3000c1f180

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts Outdated
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review exact pushed head b688ed5, including preserved contributor authorship, streaming compressed PDF extraction, symlink/FIFO race regressions, filesystem-root containment, and current-main compatibility.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b688ed540c

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review exact pushed head 340e949, including preserved external contributor attribution, bounded PDF and DOCX extraction, symlink and FIFO race protection, and deterministic macOS authentication fixture shutdown.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 340e949379

ℹ️ 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".

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review exact pushed head aaae7e1, including contributor-attributed knowledge-base ingestion, bounded compressed PDF streams before parser allocation, symlink/FIFO races, and deterministic login fixture shutdown.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aaae7e1231

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
decodeXml passed the parsed value of a numeric character reference
straight to String.fromCodePoint with no bound, so any reference above
U+10FFFF raised RangeError. That propagated out of extractDocx as
"Cannot extract text from knowledge base DOCX", which reads like a
corrupt file, and failed the whole prepareKnowledgeBase call -- one stray
reference in one document aborted the scan along with every other
knowledge-base file that was fine.

Leave a reference that cannot name a Unicode scalar value as literal
text, matching the unrecognized-named-entity fallback directly above it.
Surrogates are excluded on the same grounds: XML forbids them, and
writing one would silently encode as U+FFFD rather than throwing.

Fixes #40
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review exact pushed head 3d6bc7b, including preserved kartikshukla17 and mldangelo contributions, bounded PDF decompression before parser allocation, malformed DOCX numeric references, private source traversal, and FIFO protection.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d6bc7b051

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts Outdated
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review Please review the exact current head a3f6dd7, emphasizing the new deterministic regressions for subprocess termination, credential secrecy, configuration trust, terminal parsing, compressed document limits, and cross-platform behavior as applicable.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3f6dd7483

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review exact head 1b64527

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b64527753

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review exact head dab588e

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review exact head 2f33cb0

@chatgpt-codex-connector

Copy link
Copy Markdown

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

Reviewed commit: ee8598019d

View security finding report

ℹ️ 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ee8598019d

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review 35a7568

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 35a75686a5

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Security Review

Here are some automated security review suggestions for this pull request.

Reviewed commit: 35a75686a5

View security finding report

ℹ️ 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.

Comment thread sdk/typescript/src/knowledge-base.ts Outdated
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review 5efe24a

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5efe24a7c1

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review dd6930f

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dd6930f860

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review 139505d

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 139505ddeb

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts
@chatgpt-codex-connector

Copy link
Copy Markdown

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

Reviewed commit: 139505ddeb

View security finding report

ℹ️ 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.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review 15c4589

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 15c4589ff6

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review 00223b9

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 00223b9918

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review 602006e

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 602006ec29

ℹ️ 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".

Comment thread sdk/typescript/src/knowledge-base.ts
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
Comment thread sdk/typescript/src/knowledge-base.ts Outdated
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review 9c6053b

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9c6053b901

ℹ️ 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".

Comment on lines +1475 to +1477
selected = [...resolved.matchAll(/null|<<[\s\S]*?>>|\d+\s+\d+\s+R/gu)][
filterIndex
]?.[0];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Parse indirect DecodeParms arrays lexically

When an indirect /DecodeParms array contains a legal comment or a nested parameter dictionary, this raw non-greedy matchAll can count tokens from the comment or stop at the nested dictionary's first >>, selecting or truncating the wrong filter entry. A valid LZW/Flate object stream can then be decoded with default or incomplete predictor parameters and be rejected even though PDF.js accepts it. Fresh evidence in this head is that the new indirect-array branch applies the regex directly to the unresolved lexical structure; mask comments and extract balanced top-level array elements instead.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Security Review

Here are some automated security review suggestions for this pull request.

Reviewed commit: 9c6053b901

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.

Comment on lines +946 to +950
const length = Number(
/\/Length\s+(\d+)(?=[\s/>])/u.exec(
pdfDictionaryLexicalValues(dictionary),
)?.[1],
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Security: Resolve object-stream Length references before page discovery

pdfCrossReferenceStream() parses /Length with a direct-only regex. For a valid object stream dictionary like /Length 12 0 R, it treats 12 as the byte length instead of resolving object 12 0, so decoding the object stream fails and compressed page dictionaries are ignored. If the missed page points at an image-tagged content stream, the preflight skips that stream and PDF.js later resolves the object stream and inflates it outside the decompression budget. Reuse the indirect /Length resolver used by the main stream pass.

Useful? React with 👍 / 👎.

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