Skip to content

feat(typescript): persist findings and embeddings through the API - #660

Open
kmbroai wants to merge 4 commits into
dev/kyleb/findings-servicefrom
dev/kyleb/findings-api
Open

feat(typescript): persist findings and embeddings through the API#660
kmbroai wants to merge 4 commits into
dev/kyleb/findings-servicefrom
dev/kyleb/findings-api

Conversation

@kmbroai

@kmbroai kmbroai commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implement part 2 of the findings service: persist complete findings and embeddings through the existing HTTP endpoints and list stored findings with pagination.

Stacked on #653, with dev/kyleb/findings-service as the base so this diff contains only part 2. After #653 merges, rebase this commit onto main and retarget the PR.

Changes

  • Remove the unused provisional dedupe endpoint, result type, stub service, startup wiring, dedicated mock test, and package entry. Bulk insertion stays on its direct path; unknown requests do not call the embedder.
  • Wire POST /v1/bulk/findings to validate the existing Finding model, generate embeddings, and atomically store findings plus vectors. Return finding IDs in request order.
  • Implement GET /v1/findings with limit/offset pagination, a default page size of 50, and nextOffset.
  • Keep embedding generation and storage behind separate interfaces. Reuse the workbench database, append migration 33, share the finding upsert with CLI indexing, and invalidate vectors when stored documents change. Existing scan occurrences are preserved.
  • Add an OpenAI embedder using text-embedding-3-large with 1,536 dimensions, token-aware batching, lossless chunking, and normalized pooling. Add .env.example and document Compose credential setup and the API contracts. No CLI commands or flags are added.
  • Extend package allowlists, focused tests, and the Docker CI smoke test for the installed server, SQLite persistence, and clean shutdown.
  • Extract Docker orchestration and HTTP assertions into an SDK-typed TypeScript smoke runner explicitly included in tsconfig.json, with a small mounted TypeScript helper using Node SQLite for storage checks. Both files are explicitly type-checked. YAML only sets up Bun and invokes the runner; helpers are excluded from npm and the production image.

Testing

Verified at 2e7d40b7:

  • Focused service/embedding suites: 13 passed. The unsupported bulk-dedupe path returns HTTP 404 without calling the embedder; bulk insertion, pagination, identity conflicts, and embedding-failure rollback remain covered.
  • Full SDK suites: 1,904 passed, 28 skipped, 0 failed in each run (seeds 12345 and 3562923079).
  • pnpm run types, pnpm run format, and git diff --check: passed.
  • pnpm pack and pnpm run check:package: passed with 314 archive entries. Inspected the archive: live server modules are present; the removed dedupe module and smoke fixtures are absent. Installed public imports, NodeNext types, CLI, 114 bundled plugin files, bundled Codex, and nested-worker checks passed.
  • CI passed, including Linux, macOS, Windows, and container checks for insertion, embeddings, pagination, shutdown, and restart persistence. The local Docker smoke was not rerun because its fixed port 3000 was occupied; container CI exercised the updated script. Provider responses in checks are synthetic.

Risk and rollout

  • This preview has no API authentication. Compose continues to publish only on the host loopback interface; use an authenticated proxy before sharing access.
  • Nonempty inserts send the supplied finding JSON to the OpenAI embeddings API and require OPENAI_API_KEY or CODEX_API_KEY. Automated verification uses synthetic provider responses, not a live embedding API call.
  • Database migration 33 applies automatically. Back up existing state before upgrading. Imports update existing finding IDs; identity conflicts roll back the entire batch. Historical findings are not automatically embedded.
  • Candidate retrieval and model-based duplicate/group review remain in the next PR; this API does not expose a provisional deduplication route.

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 26, 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
🔒 Security Review Completed 2026-08-26T07:50:16.490836Z 64e1aad PR opened

Security findings

Advisory findings (1)

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

@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: 64e1aad676

ℹ️ 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 +16 to +18
if (route === "GET /v1/findings") {
console.log(route);
json(response, 200, await service.list(pagination(url.searchParams)));

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 Security: Authenticate requests before returning stored findings

When the opt-in service is running and its user visits an attacker-controlled HTTP origin capable of DNS rebinding to 127.0.0.1:3000, this new GET returns complete stored findings without any auth, Host, or Origin check. The rebound origin is same-origin, so lack of CORS does not protect the response; local start:server also uses the CLI's default workbench state, whose findings can include code evidence. The same missing control lets POSTs spend the configured embedding key or alter known documents. Loopback binding only blocks direct remote sockets. Authenticate every route and reject untrusted Host/Origin values before database or provider access.

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.

1 participant