Skip to content

fix: audit low-risk batch - orphan self-heal, tenant scope, contract alignment - #33

Merged
krishna3554 merged 1 commit into
mainfrom
fix/audit-low-risk-batch1
Sep 18, 2026
Merged

krishna3554 merged 1 commit into
mainfrom
fix/audit-low-risk-batch1

Conversation

@krishna3554

Copy link
Copy Markdown
Owner

Summary

External audit (50 tests green, tracker clean) found 12 latent issues. This PR fixes the low-risk, verifiable batch. High-severity idempotency cross-agent leak (B1) and request-size bypass (B2) are reported but left out - they need migration / design review.

Fixed

  • Orphaned idempotency 500 (B3): repository.py:create_or_get now catches RuntimeError from stale memory_id, deletes the orphan row, treats as new write instead of 500. Verified by deleting memory row directly and replaying key.
  • Tenant scoping (B4): _get_by_id now filters AND tenant_id=?, delete scopes idempotency + memories deletes by tenant. Defense-in-depth (ids are uuid4 hex, not exploitable today).
  • Recall TOCTOU (B11): main.py:recall captures single now for filter + rendering. Previously filter used one now, each result rendered with fresh now, could return expired with status expired.
  • Explain contract drift (B7): explain query now min_length=1 to match openapi/dma-v1.yaml:100-103. Empty query now 422, was 200.
  • Analyzer config footgun (B12): invalid DMA_ANALYZER_KIND now raises with allowed values instead of silent fallback to plain.
  • SDK naive expires_at (B6): client.remember now raises ValidationError for naive datetime instead of wasted 422 round-trip.
  • MCP timestamps (B5): _memory() now includes created_at/updated_at/expires_at ISO strings.

Verification

  • make check: 46 + 4 passed, ruff clean, pyright 0 errors across 5 packages
  • Manual: invalid analyzer raises, naive expires raises ValidationError, MCP payload has timestamps

Out of scope (reported separately)

  • B1 HIGH: idempotency PK not scoped to agent_id - cross-agent leak via key reuse. Needs PK migration + lookup change.
  • B2 MED: max_request_bytes only checks Content-Length header, chunked bypass. Needs actual-bytes enforcement.
  • B8 LOW: OpenAPI omits 422/413/429. Docs-only follow-up.
  • B9/B10: cursor offset normalization, metadata_depth off-by-one. One-liners for follow-up.

…alignment

- repository: self-heal orphaned idempotency rows instead of 500,
  scope _get_by_id and delete cleanup by tenant_id
- api: reuse single now in recall to avoid expiry TOCTOU,
  require min_length=1 on explain query to match OpenAPI
- config: fail fast on invalid DMA_ANALYZER_KIND with allowed values
- sdk: reject naive expires_at client-side with ValidationError
- mcp: include created_at/updated_at/expires_at in tool payloads
Copilot AI lite review requested due to automatic review settings September 16, 2026 19:03
@vercel

vercel Bot commented Sep 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
project-49zal Ready Ready Preview Sep 16, 2026 7:03pm UTC

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@krishna3554
krishna3554 merged commit c361128 into main Sep 18, 2026
4 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