Skip to content

fix: stabilize docs policy and clean-worktree validation#18

Closed
ndycode wants to merge 8 commits into
mainfrom
audit/deep-codebase-2026-03-01
Closed

fix: stabilize docs policy and clean-worktree validation#18
ndycode wants to merge 8 commits into
mainfrom
audit/deep-codebase-2026-03-01

Conversation

@ndycode
Copy link
Copy Markdown
Owner

@ndycode ndycode commented Feb 28, 2026

Fixed items

  1. Removed scoped package literal from docs/releases/v0.1.1.md to align with legacy-only scoped package mention policy.
  2. Refactored est/documentation.test.ts config precedence check to run against source exports (../lib/config.js) with explicit environment snapshot/restore, removing hidden dependency on prebuilt dist/lib/config.js.
  3. Added required .tmp entry to .gitignore so
    pm run clean:repo:check passes from a clean worktree.
  4. Upgraded hono to ^4.12.3 and added
    ollup override ^4.59.0, eliminating unexpected high/critical audit findings (GHSA-xh87-mx6m-69f3 and dev audit blocker).
  5. Hardened minimatch overrides to patched ranges (^10.2.3 global and ^9.0.7 for @typescript-eslint/typescript-estree), removing remaining high-severity dev advisory matches.
  6. Added �jv override ^6.14.0, eliminating the remaining moderate dev advisory from eslint's transitive graph.
  7. Tightened scripts/audit-dev-allowlist.js from package-name exemptions to advisory source-id allowlisting for high/critical issues.

Deferred items

  • None.

Blocked items

  • None.

Validation evidence

pm run typecheck - pass

pm run lint - pass

pm test - pass (87 files, 2071 tests)

pm run clean:repo:check - pass

pm run audit:prod - pass (0 vulnerabilities)

pm run audit:ci - pass (no unexpected high/critical findings)

pm audit --json - pass (0 vulnerabilities)

pm test -- test/documentation.test.ts - pass

pm test -- test/repo-hygiene.test.ts - pass

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

stabilized repository hygiene checks and hardened security audit policy.

  • removed scoped package literal from v0.1.1 release notes to align with legacy-only mention policy
  • refactored config precedence test to run against source exports with explicit environment snapshot/restore, eliminating build dependency
  • added .tmp to gitignore for clean-worktree validation
  • upgraded hono to ^4.12.3 and pinned rollup to ^4.59.0 to patch high/critical audit findings (authentication bypass and dev dependency vulnerabilities)
  • hardened minimatch and ajv overrides to eliminate remaining moderate/high advisories
  • critical security change: audit script now requires explicit advisory-source-id allowlisting for high/critical findings instead of blanket package exemptions, currently set to zero tolerance (empty allowlist)

all validation evidence passes per pr description. test refactoring removes hidden dist dependency and properly restores environment state. no windows filesystem or token safety risks introduced.

Confidence Score: 5/5

  • safe to merge with minimal risk
  • all changes are defensive security hardening and hygiene fixes with comprehensive validation evidence (typecheck, lint, 2071 tests, audit:ci, clean:repo:check all passing). test refactoring properly handles environment isolation. no behavioral changes to production code.
  • no files require special attention

Important Files Changed

Filename Overview
.gitignore added .tmp entry to satisfy clean-worktree check
package.json added security overrides for ajv, hono, minimatch, rollup to patch vulnerabilities
scripts/audit-dev-allowlist.js tightened from package-name exemptions to advisory-source-id allowlisting for high/critical issues
test/documentation.test.ts refactored config precedence test from subprocess to in-process dynamic import with env snapshot/restore

Last reviewed commit: 6828be7

ndycode and others added 3 commits March 1, 2026 07:47
Remove scoped package literal from v0.1.1 notes so documentation policy tests remain consistent with legacy-only mention rules.

Co-authored-by: Codex <noreply@openai.com>
Run config precedence assertions directly against source module exports with explicit env snapshot/restore, so tests pass in clean worktrees without prebuilt dist artifacts.

Co-authored-by: Codex <noreply@openai.com>
Add the exact .tmp entry expected by repo-hygiene checks to keep policy validation green.

Co-authored-by: Codex <noreply@openai.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 28, 2026

📝 Walkthrough

walkthrough

updates test execution from subprocess invocation to direct dynamic import and in-process calls, revises release note wording, adds .tmp to .gitignore, and bumps hono and adds a rollup override in package.json.

changes

Cohort / File(s) Summary
build configuration
.gitignore
added .tmp ignore pattern next to existing tmp* entry.
release notes
docs/releases/v0.1.1.md
reworded legacy package note from the scoped name to generic "prerelease package".
tests
test/documentation.test.ts
replaced execFileSync subprocess-based tests with async dynamic import of ../lib/config.js, set and restore env vars in-process (CODEX_MULTI_AUTH_DIR, CODEX_MULTI_AUTH_CONFIG_PATH, CODEX_MODE, HOME, USERPROFILE) for both test occurrences; added finally block to restore env.
package metadata
package.json
updated dependency hono from ^4.12.0 → ^4.12.3, added overrides.rollup ^4.59.0 and bumped overrides.hono to ^4.12.3.

estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

notes for review

  • missing regression tests: no new tests added that validate the refactored in-process behaviour; consider adding tests that assert original subprocess-edge cases are preserved. flag: test/documentation.test.ts:line.
  • windows edge cases: HOME vs USERPROFILE handling must be verified on windows paths and restore logic. flag: test/documentation.test.ts:line.
  • concurrency risk: modifying global env in-process can cause flakiness if tests run in parallel; confirm tests are isolated or serial. flag: test/documentation.test.ts:line.
  • module caching: dynamic import of ../lib/config.js may reuse cached module state between test runs; ensure the tests clear module cache or otherwise account for cached config. flag: test/documentation.test.ts:line.
  • verify finally blocks execute on thrown exceptions to restore env in all failure modes. flag: test/documentation.test.ts:line.

suggested labels

bug

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed title follows conventional commits format with valid type (fix) and lowercase imperative summary under 72 chars, accurately describing the main changes (docs policy alignment and gitignore cleanup).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed the pr description covers fixed items, validation evidence, and provides comprehensive context, though it lacks explicit sections matching the template structure (Summary, What Changed, Validation checklist, Docs and Governance checklist, Risk and Rollback).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch audit/deep-codebase-2026-03-01

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

Upgrade hono to ^4.12.3 to remediate GHSA-xh87-mx6m-69f3 and pin rollup to ^4.59.0 via overrides so audit:ci has no unexpected high/critical findings.

Co-authored-by: Codex <noreply@openai.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Around line 119-124: Add concise documentation explaining why the "overrides"
section pins "hono" and "rollup": update package.json to include a short comment
or add an entry to the project README/SECURITY/ADR linking to the relevant PR or
advisory that motivated the override, referencing the "overrides" object and the
specific package names "hono" and "rollup" so future maintainers can find the
rationale easily.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 0e0cc90 and 958b9dd.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • package.json
📜 Review details
🔇 Additional comments (2)
package.json (2)

123-124: rollup 4.59.0 is published and stable on npm.

the version override approach is solid for suppressing transitive dependency audit findings. pinning via overrides in package.json keeps the resolution explicit without forcing direct dependency changes.


119-119: hono upgrade properly patches ghsa-xh87-mx6m-69f3.

the advisory affects hono >= 4.12.0 and < 4.12.2, fixed in 4.12.2. bumping to 4.12.3 resolves it. both dependencies and overrides at ^4.12.3 keeps transitive resolution consistent.

Comment thread package.json
ndycode and others added 4 commits March 1, 2026 08:07
Raise minimatch overrides to patched ranges (^10.2.3 globally and ^9.0.7 for typescript-estree) to remove remaining high-severity dev audit findings while preserving toolchain compatibility.

Co-authored-by: Codex <noreply@openai.com>
Add an override for ajv ^6.14.0 to eliminate the remaining moderate dev advisory while staying within eslint's 6.x dependency expectations.

Co-authored-by: Codex <noreply@openai.com>
Replace package-name based dev audit exemptions with advisory source-id matching to tighten CI security signal and reduce accidental broad allowlisting.

Co-authored-by: Codex <noreply@openai.com>
Document why package.json overrides pin hono and rollup, including advisory context, so future maintainers can trace the security rationale without digging through old PRs.

Co-authored-by: Codex <noreply@openai.com>
@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 1, 2026

Fixed items

  1. Added persistent override rationale in SECURITY.md describing why package.json overrides pins hono and
    ollup, including advisory context for future maintainers (docs(security): document override rationale for hono and rollup, 6828be7).
  2. Resolved CodeRabbit review thread at package.json line 126 by documenting the requested rationale in the repository security documentation.

Deferred items

  • None.

Blocked items

  • CodeRabbit status check currently reports Review rate limit exceeded; this is an external service state and not remediable via repository code changes.

Validation evidence

pm run typecheck - pass

pm run lint - pass

pm test - pass (87 files, 2071 tests)

pm run audit:ci - pass

@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 1, 2026

Fixed items

  • None.

Deferred items

  • None.

Blocked items

  • None.

Validation evidence

  • gh pr checks 18 - pass
  • gh pr view 18 --json reviewDecision,mergeStateStatus,statusCheckRollup - pass (
    eviewDecision=APPROVED, mergeStateStatus=CLEAN, checks green)
  • gh api graphql (reviewThreads) - pass (all review threads resolved)

ndycode added a commit that referenced this pull request Mar 1, 2026
Source-PR: #18
Source-Branch: audit/deep-codebase-2026-03-01
Source-Head: 6828be7

Co-authored-by: Codex <noreply@openai.com>
@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 1, 2026

Fixed items

  1. Superseded by PR supersede: unify PRs #15 #16 #17 #18 #19 #20 #24 #25 into one branch #27 ( eat/unified-supersede-all-prs), which consolidates PR fix: stabilize docs policy and clean-worktree validation #18 ($branch) into the single canonical review branch.

Deferred items

  • None.

Blocked items

  • None.

Validation evidence

pm run clean:repo:check - pass

pm run audit:ci - pass

pm run typecheck - pass

pm run lint - pass

pm run build - pass

pm test - pass (92 files, 2113 tests)

@ndycode ndycode closed this Mar 1, 2026
ndycode added a commit that referenced this pull request Mar 3, 2026
@ndycode ndycode deleted the audit/deep-codebase-2026-03-01 branch March 3, 2026 12:12
ndycode added a commit that referenced this pull request Apr 6, 2026
Source-PR: #18
Source-Branch: audit/deep-codebase-2026-03-01
Source-Head: 6828be7

Co-authored-by: Codex <noreply@openai.com>
ndycode added a commit that referenced this pull request Apr 6, 2026
ndycode added a commit that referenced this pull request Apr 16, 2026
Resolves all open Dependabot alerts on package-lock.json:

- hono <4.12.14: JSX SSR HTML injection, cookie name bypass, IPv4-mapped IPv6 ipRestriction, setCookie validation, serveStatic repeated-slash bypass, toSSG path traversal (alerts #16, #18, #20, #22, #24, #26)

- vite <7.3.2: dev server WebSocket arbitrary file read, optimized deps .map path traversal, server.fs.deny query bypass (alerts #12, #13, #14)

Lockfile refreshed via npm install --package-lock-only. Typecheck, lint, and 3418/3418 tests pass.
ndycode added a commit that referenced this pull request Apr 16, 2026
Resolves all open Dependabot alerts on package-lock.json:

- hono <4.12.14: JSX SSR HTML injection, cookie name bypass, IPv4-mapped IPv6 ipRestriction, setCookie validation, serveStatic repeated-slash bypass, toSSG path traversal (alerts #16, #18, #20, #22, #24, #26)

- vite <7.3.2: dev server WebSocket arbitrary file read, optimized deps .map path traversal, server.fs.deny query bypass (alerts #12, #13, #14)

Lockfile refreshed via npm install --package-lock-only. Typecheck, lint, and 3418/3418 tests pass.
ndycode added a commit that referenced this pull request Jun 2, 2026
Completes partial fixes flagged in the third review, bumps to v2.2.1.

Real bugs (prior fixes were incomplete):
- request/fetch-helpers: isUnsupportedCodexModelForChatGpt (the handleErrorResponse
  path) now also matches NORMALIZED_UNSUPPORTED_MODEL_PATTERN, so a 400 'model not
  currently available for this chatgpt account' gets the entitlement rewrite, not
  generic error guidance (#13)
- forecast: quota-exhausted accounts are classified 'delayed' (not 'unavailable'),
  so they slipped the recommendation filter; added an explicit exhausted flag and
  excluded it, returning null when the whole pool is exhausted (#10)
- storage/record-utils: clampIndex guards NaN -> 0 (Math.trunc(NaN) propagated) (#16)
- local-client-tokens: debounce lastUsedAt persistence (60s threshold) so the
  bearer-verify hot path stops writing to disk every request; +chmod 0o700
  re-assert on the token-store dir (#12, #11)
- mcodex: relay SIGTERM/SIGINT to the spawned child so it isn't orphaned (#15)

Test quality:
- local-client-tokens: parameterized rename-retry test over ENOTEMPTY/EAGAIN/EACCES (#18)
- storage-flagged: clear the H4 deadlock-guard timer on the happy path (#17)
- storage: removeWithRetry for suite cleanup (#19)

Release:
- bump package.json + .codex-plugin/plugin.json to 2.2.1
- add docs/releases/v2.2.1.md; point docs portal + README at it

Known follow-ups (documented, deferred — need design, not rushed into a patch):
config env-path save is a single-process CAS not a true cross-process lock (#8/#9);
verifyLocalClientBearerToken read stays serialized but a fuller lease is future work;
runtime proxy routingMutex='enabled' still has a select/commit cursor race (#14)
requiring an async refactor of chooseAccount across its call sites.

Full suite: 4337 passed, 3 skipped, 0 failed; typecheck + lint clean.
ndycode added a commit that referenced this pull request Jun 2, 2026
Real bug introduced by the round-3 config lock (#18):
- the cross-process config lock fixed expiresAt once and never renewed, and
  released by unconditionally unlinking the lockfile. A save running longer than
  CONFIG_LOCK_TTL_MS could be deemed stale and stolen by another process, after
  which the original holder would delete the NEW owner's lock and reopen
  concurrent saves. Added a per-acquisition owner token (randomUUID) to the lock
  payload and a releaseConfigLockIfOwner() that compares-before-unlink, so a
  holder never deletes a lock it no longer owns. Regressions: stale-foreign-lock
  takeover (cleans only its own lock) + live-foreign-lock respected (times out,
  foreign lock untouched, no partial apply).

Nits:
- mcodex-launcher test: dropped the explicit vitest globals import (#17).
- runtime-rotation-proxy: replaced the stale 'KNOWN GAP (L4)' comment on
  chooseAccount with accurate docs — the race is closed via the reentrant
  withRoutingMutex on the hot path (#19).

Full suite: 4343 passed, 3 skipped, 0 failed; typecheck + lint clean.
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.

1 participant