fix(api): validate organizationId in the agent job/contract CRUD tools (#375) #381

Merged

james merged 1 commit from validate-job-org-in-crud-tools into main 2026-06-29 17:35:57 +00:00

Conversation 1 Commits 1 Files changed 2 +116 -14

james commented 2026-06-29 17:29:47 +00:00

Owner

Copy link

The update_job follow-up from #375 — which turned out to also close a per-user-isolation hole.

The hole

The agent create_job/update_job (and contract) tools use the shared zJobCreate/zJobUpdate schemas, which already accept organizationId (added in #375). The factory write tools passed it straight to the repo unvalidated — so an agent/MCP client could link a job to another user's organization, which then leaked that org's name to the attacker via the read LEFT JOIN. (My #379 PR note that "the generic tools don't expose organizationId" was wrong — they do, through the shared DTO. This corrects it.)

The REST routes were never affected — they validate via resolveJobOrganizationLink. Only the agent/MCP surface was.

The fix

Validate org ownership in the createRow/updateRow callbacks via the same resolveJobOrganizationLink resolver the routes use: a cross-user/missing org id is a 404 (don't leak existence); on a valid link the org's live name is snapshotted into company. Applies to both jobs and contracts, create and update. The tool descriptions now document the organizationId option.

Bonus: link_job_to_org is now fully undoable

The undo system's generic inverse for a safe_update is update_<entityType> = update_job. Now that update_job carries and validates organizationId, undoing a link (via link_job_to_org or update_job) properly clears it and restores the prior company — instead of the half-apply I flagged in #379.

Verification

• typecheck ✓ · lint ✓ · openapi:check ✓ (no spec change — agent tools aren't in the spec; the DTOs already had organizationId) · api-client check ✓ · semgrep ✓ 0 findings.
• test against both engines (ephemeral Postgres): 1272 passed, 0 skipped — incl. new dual-engine tests: create_job/update_job reject a cross-user organizationId (no write, no leak), and a link made via update_job round-trips through undo (clears the link + restores the prior company).

Part of #375.

🤖 Generated with Claude Code

The `update_job` follow-up from #375 — which turned out to also close a **per-user-isolation hole**. ## The hole The agent `create_job`/`update_job` (and contract) tools use the shared `zJobCreate`/`zJobUpdate` schemas, which **already accept `organizationId`** (added in #375). The factory write tools passed it straight to the repo **unvalidated** — so an agent/MCP client could link a job to **another user's organization**, which then leaked that org's name to the attacker via the read `LEFT JOIN`. (My #379 PR note that "the generic tools don't expose organizationId" was wrong — they do, through the shared DTO. This corrects it.) The REST routes were never affected — they validate via `resolveJobOrganizationLink`. Only the agent/MCP surface was. ## The fix Validate org ownership in the `createRow`/`updateRow` callbacks via the **same** `resolveJobOrganizationLink` resolver the routes use: a cross-user/missing org id is a **404** (don't leak existence); on a valid link the org's live name is snapshotted into `company`. Applies to both jobs and contracts, create and update. The tool descriptions now document the `organizationId` option. ## Bonus: `link_job_to_org` is now fully undoable The undo system's generic inverse for a `safe_update` is `update_<entityType>` = `update_job`. Now that `update_job` carries **and validates** `organizationId`, undoing a link (via `link_job_to_org` or `update_job`) properly **clears** it and restores the prior company — instead of the half-apply I flagged in #379. ## Verification - `typecheck` ✓ · `lint` ✓ · `openapi:check` ✓ (no spec change — agent tools aren't in the spec; the DTOs already had `organizationId`) · `api-client check` ✓ · semgrep ✓ 0 findings. - `test` against **both engines** (ephemeral Postgres): **1272 passed, 0 skipped** — incl. new dual-engine tests: `create_job`/`update_job` reject a cross-user `organizationId` (no write, no leak), and a link made via `update_job` round-trips through undo (clears the link + restores the prior company). Part of #375. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james added 1 commit 2026-06-29 17:29:47 +00:00

fix(api): validate organizationId in the agent job/contract CRUD tools (#375) ... 481fa0d56b

The agent create_job/update_job (+ contract) tools use the shared
zJobCreate/zJobUpdate schemas, which already accept organizationId (#375).
The factory write tools passed it straight to the repo UNVALIDATED — so an
agent/MCP client could link a job to another user's organization, which
then leaked that org's name through the read join. (My #379 note that the
generic tools "don't expose organizationId" was wrong — they do, via the
shared DTO.)

Validate org ownership in the createRow/updateRow callbacks via the same
resolveJobOrganizationLink resolver the REST routes use: a cross-user or
missing org id is a 404 (don't leak existence); on a valid link the org's
live name is snapshotted into company. Applies to both jobs and contracts.

Bonus: this makes link_job_to_org fully undoable — the undo system's
inverse (update_job) now carries + validates organizationId, so undoing a
link clears it (and restores the prior company) instead of half-applying.

Dual-engine tests: create_job/update_job reject a cross-user organizationId
(no write, no leak); a link made via update_job round-trips through undo.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

forgejo-actions commented 2026-06-29 17:35:11 +00:00

Copy link

📊 Test coverage

Patch coverage: no testable lines changed.

Overall (app/, lib/, db/, excluding UI per ADR-0019):

Metric	Value	Soft target
Lines	79.6% ✅	≥ 50%
Branches	71.5% ⚠️	≥ 75%
Functions	81.2%	informational

Soft thresholds per ADR-0019. Coverage is informational and does not block merge.

<!-- coverage-comment --> ## 📊 Test coverage **Patch coverage:** no testable lines changed. **Overall** (`app/`, `lib/`, `db/`, excluding UI per ADR-0019): | Metric | Value | Soft target | |---|---|---| | Lines | 79.6% ✅ | ≥ 50% | | Branches | 71.5% ⚠️ | ≥ 75% | | Functions | 81.2% | informational | Soft thresholds per [ADR-0019](docs/adr/0019-coverage-soft-targets.md). Coverage is informational and does not block merge.

james merged commit 91e50a74fd into main 2026-06-29 17:35:57 +00:00

james deleted branch validate-job-org-in-crud-tools 2026-06-29 17:35:58 +00:00

james referenced this pull request from a commit 2026-06-29 17:35:58 +00:00

Merge pull request 'fix(api): validate organizationId in the agent job/contract CRUD tools (#375)' (#381) from validate-job-org-in-crud-tools into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area:auth

Authentication and authorization

  

area:ci

Forgejo Actions, build, test, security, release

  

area:db

Database abstraction, migrations, persistence

  

area:infra

Containerization, deployment, foundational tooling

  

area:native

React Native app (Android + Linux Flatpak)

  

area:pwa

PWA / web frontend

  

area:service

Backend service, API routes, persistence

  

epic

High-level tracker; links several feature tickets

  

feature

A user-facing feature

  

foundation

Foundational work that other tickets build on

No labels

area:auth

area:ci

area:db

area:infra

area:native

area:pwa

area:service

epic

feature

foundation

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

james/carol!381

No description provided.