Skip to content

Fix/sdk typescript security - #1283

Merged
Neelpatel1604 merged 6 commits into
mainfrom
fix/sdk-typescript-security
Jun 29, 2026
Merged

Neelpatel1604 merged 6 commits into
mainfrom
fix/sdk-typescript-security

Conversation

@Neelpatel1604

@Neelpatel1604 Neelpatel1604 commented Jun 29, 2026 •

Copy link
Copy Markdown
Member
  • TypeScript SDK: Bump to 0.2.3; upgrade @hey-api/openapi-ts (^0.99.0) and vitest (^4.1.9) to fix npm audit issues (handlebars, tar, vite/esbuild)
  • Conflicts UI: Rebuild conflict rendering with DOM APIs (textContent / createElement) instead of innerHTML to fix CodeQL XSS alerts
  • CI: Add permissions: contents: read to ci.yml for least-privilege GitHub Actions token

Summary by CodeRabbit

  • Bug Fixes

    • Improved Conflict Resolution page rendering by switching to DOM-based UI construction for the banner, empty state, conflict cards, manual resolution controls, and actions.
    • Updated the error path to render messages more consistently without replacing the container via large HTML strings.
  • Chores

    • Restricted CI workflow token access to read-only contents.
    • Refreshed TypeScript SDK tooling (including OpenAPI generator settings, dev dependency bumps, and requiring Node >= 20), updated TS lib to include DOM, and refreshed the OpenAPI spec version/contract text and numeric validation constraints.

- Bump package version to 0.2.3
- Upgrade @hey-api/openapi-ts to ^0.99.0
- Upgrade vitest to ^4.1.9
- Update package-lock.json with new dependencies and versions
- Added read permissions for contents in the CI workflow.
- Implemented functions in the UI to handle and display conflict information, including banners and metadata for conflicts in memories.
@Neelpatel1604
Neelpatel1604 requested a review from het0814 June 29, 2026 15:06
@coderabbitai

coderabbitai Bot commented Jun 29, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@Neelpatel1604, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ce28e13d-08b4-43b6-86e8-0d95f0050b31

📥 Commits

Reviewing files that changed from the base of the PR and between 06c3abb and 20b11f0.

📒 Files selected for processing (1)
  • .github/workflows/publish.yml
📝 Walkthrough

Walkthrough

The conflicts page now renders through DOM helper functions instead of HTML strings. The CI workflow restricts token permissions, and the TypeScript SDK updates generator settings, compiler DOM typings, package versions, and OpenAPI schema literals.

Changes

Conflicts Page DOM Refactor

Layer / File(s) Summary
DOM rendering helpers and loader wiring
memanto/app/ui/static/index.html
Adds helpers for the conflicts banner, empty state, metadata blocks, action buttons, conflict cards, and list rendering, then updates loadConflicts() to use them and to render errors with created DOM nodes.

CI and TypeScript SDK Updates

Layer / File(s) Summary
Workflow and SDK toolchain
.github/workflows/ci.yml, sdks/typescript/package.json, sdks/typescript/openapi-ts.config.ts, sdks/typescript/tsconfig.json
Adds permissions: contents: read to the workflow, upgrades @hey-api/openapi-ts and vitest, removes format: "prettier" from the generator output config, and adds DOM to compilerOptions.lib.
OpenAPI spec normalization
sdks/typescript/openapi.json
Updates the spec version, adjusts several endpoint descriptions, and changes multiple numeric validation literals to floating-point values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • moorcheh-ai/memanto#706: Both PRs modify the Conflicts UI in memanto/app/ui/static/index.html, including the conflict loading/rendering path.

Suggested reviewers

  • het0814

Poem

🐇 I hopped through DOM nodes, neat and bright,
No stringy cards in conflict sight.
Read-only tokens guard the gate,
And TypeScript learns DOM just right.
New tools hum softly through the night.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is related to the PR, but it is vague and awkwardly phrased, so it does not clearly communicate the main change. Rename it to a clear summary like 'Harden TypeScript SDK security and refactor conflict rendering'.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sdk-typescript-security

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.

- Removed the 'format' option from openapi-ts.config.ts.
- Added 'DOM' to the 'lib' array in tsconfig.json for improved type support.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@memanto/app/ui/static/index.html`:
- Around line 4457-4459: The manual resolution submit handler in
conflictActionBtn currently passes textarea.value directly to resolveConflict,
which allows empty content to reach the backend. Add a client-side non-empty
check in the Submit Manual Resolution callback before calling resolveConflict,
and block submission with a clear prompt or early return when textarea.value is
blank. Use the existing resolveConflict and conflictActionBtn flow to locate the
fix.

In `@sdks/typescript/package.json`:
- Line 35: The package is declaring a Node 18 engine range while the Vitest
dependency in this package now requires Node 20 or newer. Update the
engines.node constraint in sdks/typescript/package.json to match the runtime
required by vitest@4.1.9, or otherwise downgrade Vitest to a major that supports
Node 18; keep the change aligned with the existing vitest/config and test API
usage.
- Line 31: The TypeScript OpenAPI generator config still uses the old plugin
array shape, so update `sdks/typescript/openapi-ts.config.ts` to match the 0.99
API. Keep `output.path` as-is, but change the `plugins` entry from the string
form to a plugin object in the `@hey-api/client-fetch` configuration, using the
generator’s expected `{ name: "`@hey-api/client-fetch`", ... }` shape so the
config works with `@hey-api/openapi-ts` 0.99.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 56a37e6a-dbfd-486d-aed2-606a8a40cc03

📥 Commits

Reviewing files that changed from the base of the PR and between d922b7a and 9e6e4f4.

⛔ Files ignored due to path filters (1)
  • sdks/typescript/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • memanto/app/ui/static/index.html
  • sdks/typescript/package.json

Comment thread memanto/app/ui/static/index.html
Comment thread sdks/typescript/package.json
Comment thread sdks/typescript/package.json
- Changed version from 0.2.4.dev12+g0e7dd4f.d20260623 to 0.0.0.dev0.
- Updated descriptions in several API endpoints to replace hyphens with em dashes for consistency.
- Adjusted numeric types in schema definitions to use floating-point values for maximum and minimum constraints.
- Added validation to ensure that manual replacement content is not empty before submission.
- Display a warning toast and focus on the textarea if the content is blank.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
sdks/typescript/openapi.json (1)

3119-3168: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Fix the date-time vs YYYY-MM-DD mismatch.

RecallAsOfRequest.as_of and RecallChangedSinceRequest.since are still declared as format: "date-time", but the descriptions now promise YYYY-MM-DD is accepted. Either widen the schema to allow date-only input or tighten the descriptions to RFC3339 timestamps only.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@sdks/typescript/openapi.json` around lines 3119 - 3168, The
RecallAsOfRequest.as_of and RecallChangedSinceRequest.since schema entries still
declare format: date-time while their descriptions say YYYY-MM-DD is accepted,
so align the OpenAPI definition with the actual supported input. Update the
relevant properties in openapi.json by either changing the schema to allow
date-only strings in addition to full datetimes, or by narrowing the
descriptions to RFC3339-only timestamps; keep the documentation and validation
behavior consistent for these request models.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@sdks/typescript/openapi.json`:
- Around line 3119-3168: The RecallAsOfRequest.as_of and
RecallChangedSinceRequest.since schema entries still declare format: date-time
while their descriptions say YYYY-MM-DD is accepted, so align the OpenAPI
definition with the actual supported input. Update the relevant properties in
openapi.json by either changing the schema to allow date-only strings in
addition to full datetimes, or by narrowing the descriptions to RFC3339-only
timestamps; keep the documentation and validation behavior consistent for these
request models.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 814198e4-473e-4277-aaf3-b63172f48cc2

📥 Commits

Reviewing files that changed from the base of the PR and between 774247f and a9ba892.

📒 Files selected for processing (1)
  • sdks/typescript/openapi.json

@Neelpatel1604
Neelpatel1604 requested a review from Xenogents June 29, 2026 15:20
- Introduced a new job in the CI workflow to publish the TypeScript SDK to npm upon release.
- Configured steps for checking out code, setting up Node.js, installing dependencies, versioning, generating client code, type checking, testing, building, and publishing.
- Ensured the workflow triggers only on versioned releases.
@Neelpatel1604
Neelpatel1604 merged commit bce6ccc into main Jun 29, 2026
8 checks passed
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