Skip to content

fix(connect): bound catalog response lifetime - #463

Draft
luvs01 wants to merge 76 commits into
Devfrom
codex/fix-vulnerability-in-catalog-downloads
Draft

fix(connect): bound catalog response lifetime#463
luvs01 wants to merge 76 commits into
Devfrom
codex/fix-vulnerability-in-catalog-downloads

Conversation

@luvs01

@luvs01 luvs01 commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Motivation

  • A catalog fetch replaced the prior whole-request deadline with only an inactivity timer that is reset on every non-empty chunk, allowing a malicious hub to keep a download alive indefinitely by trickling bytes; this can retain sockets and strand issued service tokens if the CLI is force-terminated during the stalled await.

Description

  • Add a capped absolute body lifetime to catalog downloads by supplying an overall AbortSignal.timeout to body consumption while preserving the existing inactivity deadline semantics for progress windows.
  • Propagate an optional signal through boundedText into readBoundedResponseBytes so the bounded reader can deterministically cancel and release the response stream when the total lifetime expires.
  • Cancel response bodies prior to throwing on unexpected 304 and non-OK HTTP responses to avoid leaving an uncancelled stream in those early-return branches.
  • Add focused regression tests in tests/clients/remote-catalog.test.ts that assert (a) a slow-drip response is now killed by the total deadline and (b) HTTP error bodies are cancelled before the call rejects.

Testing

  • Ran the focused catalog suite with bun test tests/clients/remote-catalog.test.ts and it passed (55 tests, 0 failures).
  • Ran bun run typecheck and bun run privacy:scan and both completed successfully.
  • Ran the full repository test run (bun run test) for context; the overall suite completed but reported unrelated environment-sensitive/timing failures (WebSocket relay timing, Lab automation dispatch, and Unix shim timeout tests), while the focused catalog tests added here passed.

Codex Task

github-actions Bot and others added 30 commits September 7, 2026 06:31
…#3812)

Move dev to 2.46.0 before publishing 2.45.0.

The owner explicitly authorized this release train and maintainer integration. Official pre-move workflow34061256762 passed the unused-version and release-version-line checks. This PR changes package.json only. Frozen release source cf9f662 is being validated by Cross-platform CI34061274315 and Service lifecycle34061276621; those pending runs are not claimed passing. Release publication remains gated on candidate and exact release-branch evidence.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Record scoped roadmap and independent plan audit. Local suites are excluded by owner instruction; final remote CI will verify the combined candidate.
Carry PR lidge-jun#3815 through 76e07d1.
Preserve original contributor work and its bounded retention, signature-only
reasoning and documentation follow-ups. Remote combined validation follows;
local suites intentionally not run under maintainer instruction.

Co-authored-by: SB Yoon <44089734+yansigit@users.noreply.github.com>
Co-authored-by: Yumi <automation@sbyoon.com>
Carry PR lidge-jun#3816 at d5e0a9a.
Keep original frames on the proxy inspection branch and scope projection
to the existing Grok HTTP/SSE client marker.

Co-authored-by: Danh Thanh <danhthanh418@gmail.com>
Record source-delta audits, manual PR delivery and final-head CI strategy.

Co-authored-by: Éverton Toffanetto <evertondgn@hotmail.com>
Co-authored-by: Zig Zag <shafishahin786@proton.me>
Co-authored-by: 투린 <me@turin.my>
Carry architecture context for lidge-jun#3641 and lidge-jun#3733. Live BigModel discovery remains deferred; local suites are not run.

Co-authored-by: jamespan <panjiabang@gmail.com>

Co-authored-by: Chanhee Lee <hiddenest12@gmail.com>
Carry the remaining teardown fix from lidge-jun#3383 (51726d2) and verify cleanup ordering with a held asynchronous runner. Final combined CI will validate the stack; no local suite was run.

Co-authored-by: x3M3x <amroeid1999@gmail.com>
Use the last event field and preserve significant whitespace while retaining independent JSON control-type filtering. Add discriminator, order, reset, and preservation regressions to the existing Responses test file. Tests were authored but not run; validation is delegated to final combined remote CI.

Co-authored-by: Danh Thanh <danhthanh418@gmail.com>
Add one synthetic complete send_message_to_thread envelope after a real tool pair. Cover ordinary responses, stored-ID continuation, v2 compaction_trigger and v1 compact; assert upstream content, order and pairing plus compact output contracts.

Coverage motivated by issue lidge-jun#3807 reports from @DaveW001 and @stephen-drew, using the narrowed envelope contract documented in lidge-jun#3735. These are synthetic fixtures, not captured reporter requests; no original source patch is copied.

Validation: git diff --check passed. Tests, typecheck and build NOT RUN by explicit instruction. Production code and missing-call-id guards are unchanged.
…ip ci]

Report declared wire length, observed lower bounds, and decoded sizes without reading rejected payloads further or changing admission and retry policy. Follow up on lidge-jun#3573; thanks to @nowhere1975 for the content-free incident measurements. The issue remains open for exact compact-attempt evidence. Independent source review passed; final remote CI pending.
…n#3641

Narrowed carry of jamespan’s PR lidge-jun#3641, source origin/axis2-source-3641 at b675d83. Reconstruct the separate Responses preset using only the two models in the official Codex example: https://docs.bigmodel.cn/cn/coding-plan/tool/codex.md (checked 2026-09-07).

Keep liveModels false and preserve custom destinations and Responses reasoning replay. Map exact context windows, effort ladders, max defaults, summary support, and text modalities. Do not carry model-discovery.ts or envelopeKey/idKey; Flash Responses metadata and live discovery remain unverified.

Add consumer metadata and custom-transport collision regressions; document the static roster and existing Codex export policy (compatibility ultra on GLM-5.3, omitted default field on Turbo’s empty ladder). Validation: git diff --check passed. Tests, typecheck, lint, and builds intentionally not run per worker scope; parent final CI owns execution.

Co-authored-by: jamespan <panjiabang@gmail.com>
…t [skip ci]

Carry PR lidge-jun#3627 from f699ec7,
using merge base cf9f662.
Keep the source runtime changes and existing Astra normalization intact.
Qualify external-name restoration in all four provider docs and catalog SOT.
Add Astra external-edit coverage and real retained/convergence writer coverage.

Local tests, typecheck and builds NOT RUN by owner mandate.
Static diff inspection and git diff --check passed; remote CI belongs to parent.

Co-authored-by: Éverton Toffanetto <evertondgn@hotmail.com>
Carry the net diff of lidge-jun#3780 at 9b873e6. Strengthen multi-record JSON parity, escaping, and conflicting-flag coverage; synchronize all seven translated CLI pages and clarify consumer-side processing.

Local tests, typecheck, and builds NOT RUN by owner mandate. Regenerated the capability surface and inspected the static diff. Lower-layer CI is owner-requested deferred evidence, not passing evidence; final combined CI belongs to the integration owner.

Co-authored-by: 투린 <me@turin.my>
Exercise the source-build contract from lidge-jun#3421/lidge-jun#3604 and separate-home persistence from lidge-jun#3747 using disposable Docker resources. Preserve original token and both named volumes through container replacement; verify admission without upstream inference. Independent source/security review completed; actual Docker networking and runtime acceptance remain pending final CI.

Co-authored-by: Buseong Kim <flight@skyline23.com>

Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
Carry the net diff from PR lidge-jun#2716 at 93ed440.
Reconcile confirmed saved/reset receipts, including saved:true errors, with the
editor draft, current label and reset availability. Preserve reset intent on
retry and retry only the list read after a successful mutation and failed read.

Reuse createBoundedFetch for a single 60-second write-and-refresh budget.
Timeouts retain the draft, release the modal lock and leave persistence unknown;
retry reads current state before another mutation. Keep global fetch unchanged.

Add focused regression coverage, nine-locale recovery copy and workflow docs.
Local tests, typecheck, build and browser smoke NOT RUN by owner mandate.
Static diff inspection only; parent owns remote CI and browser verification.

Co-authored-by: Zig Zag <shafishahin786@proton.me>
…lines [skip ci]

Carry and refine lidge-jun#3809: observe each request-bound physical response, preserve probe clocks and model-specific windows, and retain valid multi-day upstream reset deadlines. Preserve credential ownership and skip unprovable observations. Runtime checks are deferred to the final cumulative hosted CI at owner request; no local suite was run.

Co-authored-by: Éverton Toffanetto <evertondgn@hotmail.com>
Preserve empty signed and redacted-only replay items, keep signature updates within their source thinking block, and emit opaque blocks in order. Retain hidden-summary policy and document deferred Claude hidden-text replay and live/cache claims. Add exact-array synthetic round-trip coverage; local tests and typecheck intentionally not run, pending parent combined remote CI.

Co-authored-by: SB Yoon <44089734+yansigit@users.noreply.github.com>
Co-authored-by: Yumi <automation@sbyoon.com>
Keep an earlier confirmed save/reset receipt when retrying the same value and
convergence fails with an ordinary HTTP error. Do not replace the saved snapshot
from an unconfirmed response; retain the pending reset intent for another retry.

Treat transport and response-body failures without a usable receipt as unknown
outcomes, hide the stale current name, and make Retry/Enter read-only. Keep
known unpersisted HTTP failures on the existing editable-draft path.

Add regressions for reset -> saved:true -> HTTP failure -> success and persisted
save/reset followed by rejected transport or response-body reads without abort.
Local tests/typecheck/build NOT RUN by owner mandate; static diff check only.

Co-authored-by: Zig Zag <shafishahin786@proton.me>
Skip undocumented HTTP model-list validation through the existing unknown key-validation policy. Add a zero-fetch regression.

For explicitly empty model/provider effort ladders, omit only the Responses effort field and retain reasoning summaries. Preserve omitted requests, nonempty model override precedence, unknown/non-rankable ladders, and forward behavior. Keep the documented Turbo max metadata without adding a selectable tier or injecting a wire default. Existing __omit__ mappings and noReasoningModels policy are unchanged.

Add outbound serialization cases for omitted, max and ultra effort, summary preservation, input immutability, and unchanged consumer paths. Regenerate model metadata; the generated delta is only the BigModel Responses to zai alias.

Validation: bun run generate:model-metadata and git diff --check passed. No tests, typecheck, lint or builds run; commit hooks skipped to honor that restriction. Main final CI and separate independent review remain required.

Co-authored-by: jamespan <panjiabang@gmail.com>
First CI run 34064726690 reports text,image rather than text for both BigModel models at provider-registry-parity.test.ts:471; the Turbo effort array remains empty. Source tracing confirms applyProviderConfigHints uses isModelVisionSidecarConsumer, also used by the runtime vision planner, to expose image attachment support for configured text-only models.

Keep the official text-only registry declarations and assert them independently before checking the bridge-enriched model and final catalog modalities. Preserve exact Turbo empty effort expectations and all generic modality behavior. Clarify that image handling requires the existing available, enabled vision sidecar rather than native BigModel image support.

Validation: supplied CI failure log and source contract reviewed; git diff --check passed. No local tests, typecheck, lint or builds run. Commit hooks skipped per execution restriction. Main final CI and independent review remain required.

Co-authored-by: jamespan <panjiabang@gmail.com>
Browser review found an isolated Korean ending at 390px. Balance the short helper sentence without fixed line breaks or changing the existing layout.

Co-authored-by: Zig Zag <shafishahin786@proton.me>
Distinguish termination entry from terminal delivery so closure-time overflow can release thinking and emit the bounded error without retrying closure. Prioritize collected errors over unfinished block serialization. Add eight real-budget closure-only overflow cases for EOF, failure, completion and incomplete terminals, including shared-budget collection. Tests and typecheck intentionally not run; parent owns final combined remote CI.

Co-authored-by: SB Yoon <44089734+yansigit@users.noreply.github.com>
Co-authored-by: Yumi <automation@sbyoon.com>
…ing [skip ci]

CI run 34064754947 built and started the image, then proved internal-only networking suppressed host port publication. Keep actual Compose networking and seed a checked loopback-only provider into disposable state before startup. Verify loaded configuration and hashes through recreation.

Co-authored-by: Buseong Kim <flight@skyline23.com>

Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
Combined Linux CI at c721b94 reported six messages where the seed fixtures expected five. Non-OpenAI chat translation prepends system tool-catalog guidance while compaction removes context.tools first. Explicitly require one system prefix advertising read_value on ordinary and stored-ID turns, and none on compact turns. Keep exact total length, ordered history content, original tool pairing and compact output assertions.

Follow-up to synthetic lidge-jun#3807 coverage motivated by @DaveW001 and @stephen-drew; no original source patch copied. Source-only review and git diff --check passed. Local tests, typecheck and build NOT RUN by instruction.
Match the canonical user string observed at parent combined head c721b94 while preserving exact assistant block arrays. Capture closure-overflow output before collecting under the same unreset budget, separating concurrent ingestion pressure from closure-only failure. Assert all text, one bounded error, no success terminal, exact 32768-byte overflow boundary and no second overflow. No local tests or typecheck run.

Co-authored-by: SB Yoon <44089734+yansigit@users.noreply.github.com>
Co-authored-by: Yumi <automation@sbyoon.com>
The Docker diagnostic failed because loadConfig correctly rejects private destinations unless explicitly allowed. A bounded remote fixture reproduced the fallback and passed with the test-only flag. Validate the fixture before startup and report fixed seed-phase codes without exposing child errors.

Co-authored-by: Buseong Kim <flight@skyline23.com>

Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
Replace effect-driven draft synchronization with a guarded render-state
adjustment when the parent supplies a new confirmed model snapshot. Keep the
dialog mounted, retaining focus refs, pending state and request errors. Ordinary
typing and catalog polling do not replace the editor snapshot.

Local tests, lint, typecheck and build NOT RUN by owner mandate.
Static diff inspection only; final CI and independent review remain parent-owned.

Co-authored-by: Zig Zag <shafishahin786@proton.me>
Carry source fa8b488 onto current dev. Local test, typecheck, lint and build execution deferred to combined remote CI per task scope.

Co-authored-by: Chanhee Lee <hiddenest12@gmail.com>
lidge-jun and others added 18 commits September 7, 2026 09:49
…-final

ci: gate real Docker build startup and recreation
Integrate verified axis-five contribution. Feature head f51ec24 passed all25 CI jobs; prospective merged tree85c9b258 passed370 focused tests on isolated Linux. Owner-authorized admin integration and lower-layer CI deferral are recorded in the PR.

Co-authored-by: Éverton Toffanetto <evertondgn@hotmail.com>
Integrate verified axis-five contribution. Feature head f51ec24 passed all25 CI jobs; prospective merged tree85c9b258 passed370 focused tests on isolated Linux. Owner-authorized admin integration and lower-layer CI deferral are recorded in the PR.

Co-authored-by: 투린 <me@turin.my>
Integrate verified axis-five contribution. Feature head f51ec24 passed all25 CI jobs; prospective merged tree85c9b258 passed370 focused tests on isolated Linux. Owner-authorized admin integration and lower-layer CI deferral are recorded in the PR.

Co-authored-by: Zig Zag <shafishahin786@proton.me>
Record credited admin landings, successful feature CI, exact merged-tree focused checks, and the unresolved Mac timing/stall diagnosis.

Co-authored-by: Éverton Toffanetto <evertondgn@hotmail.com>
Co-authored-by: 투린 <me@turin.my>
Co-authored-by: Zig Zag <shafishahin786@proton.me>
Archive the completed axis-five unit and preserve its exact verification and attribution record. Documentation-only; runtime code is unchanged.

Co-authored-by: Éverton Toffanetto <evertondgn@hotmail.com>
Co-authored-by: 투린 <me@turin.my>
Co-authored-by: Zig Zag <shafishahin786@proton.me>
Owner-authorized admin merge of a documentation-only privacy-scan repair. Privacy scan and diff check passed; no local test suite ran. Author names and historical commit attribution are preserved. No runtime or scanner-policy changes.
Use private BigInt stat identities so distinct legal 64-bit file IDs are not collapsed by Number conversion. Preserve all path/link/hardlink and directory replacement checks, manifest bounds, public IO types and serialization. Add semantic high-ID and native link controls with fixture-reachability checks. No local suite was run; the earlier Windows incident IDs remain unmeasured.
Owner-authorized admin merge of axis1 PR lidge-jun#3825 at exact head d3c70f9.

Final combined CI: https://github.com/lidge-jun/opencodex/actions/runs/34074350604 at 9470fdb.
The combined candidate includes the entire stack and dev bf85e67; its tree is 90a7511. Independent source/security review and regression evidence are recorded in the PRs.

The owner explicitly prohibited local suites, requested --no-verify pushes, final cumulative CI before any diagnostic lower runs, ordinary (non-native) stacking, and admin merge. Lower skipped/cancelled checks are not reported as passing. This admin action uses that explicit review/lower-CI exception. Merge commits preserve all original commits and contributor trailers, including Éverton Toffanetto's Co-authored-by in f215f79. No release or deployment.
Owner-authorized admin merge of axis1 PR lidge-jun#3826 at exact head 872f0e5.

Final combined CI: https://github.com/lidge-jun/opencodex/actions/runs/34074350604 at 9470fdb.
The combined candidate includes the entire stack and dev bf85e67; its tree is 90a7511. Independent source/security review and regression evidence are recorded in the PRs.

The owner explicitly prohibited local suites, requested --no-verify pushes, final cumulative CI before any diagnostic lower runs, ordinary (non-native) stacking, and admin merge. Lower skipped/cancelled checks are not reported as passing. This admin action uses that explicit review/lower-CI exception. Merge commits preserve all original commits and contributor trailers, including Éverton Toffanetto's Co-authored-by in f215f79. No release or deployment.
Owner-authorized admin merge of axis1 PR lidge-jun#3827 at exact head 2e8ef03.

Final combined CI: https://github.com/lidge-jun/opencodex/actions/runs/34074350604 at 9470fdb.
The combined candidate includes the entire stack and dev bf85e67; its tree is 90a7511. Independent source/security review and regression evidence are recorded in the PRs.

The owner explicitly prohibited local suites, requested --no-verify pushes, final cumulative CI before any diagnostic lower runs, ordinary (non-native) stacking, and admin merge. Lower skipped/cancelled checks are not reported as passing. This admin action uses that explicit review/lower-CI exception. Merge commits preserve all original commits and contributor trailers, including Éverton Toffanetto's Co-authored-by in f215f79. No release or deployment.
Owner-authorized admin merge of axis1 PR lidge-jun#3842 at exact head b29bbb4.

Final combined CI: https://github.com/lidge-jun/opencodex/actions/runs/34074350604 at 9470fdb.
The combined candidate includes the entire stack and dev bf85e67; its tree is 90a7511. Independent source/security review and regression evidence are recorded in the PRs.

The owner explicitly prohibited local suites, requested --no-verify pushes, final cumulative CI before any diagnostic lower runs, ordinary (non-native) stacking, and admin merge. Lower skipped/cancelled checks are not reported as passing. This admin action uses that explicit review/lower-CI exception. Merge commits preserve all original commits and contributor trailers, including Éverton Toffanetto's Co-authored-by in f215f79. No release or deployment.
Owner-authorized admin documentation closeout. The four runtime PRs and exact combined CI/tree proof are recorded in the unit. Privacy scan and staged diff checks passed; no local application suites or typecheck ran. This merge changes only devlog records and preserves all runtime code and attribution.
[WRONG BRANCH] chore(release): promote verified 2.46.0 to main
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T07:13:47.000354Z 8776055 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: aef86871-9711-4945-b0a3-f7b4d2f54ca4


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

⏳ DRAFT

  • wrong target branch (main); retarget to dev.

What to do

  • Retarget this PR to dev — all contributions go to dev.

Its title has been prefixed with [WRONG BRANCH].
This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.

@github-actions github-actions Bot changed the title fix(connect): bound catalog response lifetime [WRONG BRANCH] fix(connect): bound catalog response lifetime Sep 7, 2026
@github-actions
github-actions Bot marked this pull request as draft September 7, 2026 07:10

@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: 877605534e

ℹ️ 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 src/client/hub-client.ts
inactivityTimeoutMs: safeTimeout(options.timeoutMs),
// Permit active catalog transfers to span multiple inactivity windows,
// while retaining the client's established maximum request lifetime.
signal: AbortSignal.timeout(Math.min(inactivityTimeoutMs * 24, 120_000)),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep the deadline compatible with supported catalog sizes

For any default or CLI timeout of at least five seconds, this expression imposes an unconfigurable 120-second body limit. MAX_REMOTE_CATALOG_BYTES permits 256 MiB, and src/server/catalog-download.ts documents a supported 2,000-row catalog at roughly 92 MiB; over a connection below about 6.5 Mbit/s, that valid catalog takes more than 120 seconds despite continuously arriving bytes and now fails as unreachable. This also contradicts the documented --catalog-timeout inactivity semantics, so use a total bound compatible with supported payloads or expose and document a separate total deadline.

AGENTS.md reference: AGENTS.md:L371-L372

Useful? React with 👍 / 👎.

Comment thread src/client/hub-client.ts
throw new HubClientError("catalog_unexpected_304", "Hub answered 304 to an unconditional catalog request", 304);
}
if (!response.ok) {
try { await response.body?.cancel(); } catch { /* best effort */ }

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 Do not await untrusted response cancellation

When an injected fetch implementation returns a non-OK response whose stream's underlying cancel() promise never settles, this newly added await leaves downloadClientCatalog pending forever instead of throwing catalog_http_*; a legal ReadableStream cancel algorithm can return such a promise. Cancel on a fire-and-forget basis with rejection observation, as readBoundedResponseBytes already does, so cleanup cannot become another unbounded transport wait.

AGENTS.md reference: src/AGENTS.md:L17-L17

Useful? React with 👍 / 👎.

@luvs01 luvs01 changed the title [WRONG BRANCH] fix(connect): bound catalog response lifetime fix(connect): bound catalog response lifetime Sep 7, 2026
@luvs01
luvs01 changed the base branch from main to Dev September 7, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants