feat(operations): add a put operation, and fix the target-database authorization mismatch

[dawsontoth]

Two changes on one branch, reviewable in order: a put operation for the operations API, and an authorization fix found while building it.

commit	what
1–4	superseded by 5 — kept for review history, squash on merge
5	put: create-or-replace, replacing the earlier full_record flag and __unset__ directive
—	security: resolve the target database identically in authorization and handlers

---

put

{ "operation": "put", "database": "dev", "table": "dog",
  "records": [{ "id": 1, "dog_name": "Penny" }] }

The stored record becomes exactly that, so an attribute the request omits is removed. Creates a missing record, replaces an existing one.

Why it's needed. There is currently no way to remove an attribute from a record through the operations API. update and upsert both land on Table.patch for a record that already exists (ResourceBridge.upsertRecords), which merges: an omitted attribute keeps its stored value, and null stores a null. A full replace existed only over REST (PUT /Table/id → resource.put), which a client speaking the operations API can't reach — and can't reach at all through a proxied operations endpoint. Reported downstream as HarperFast/studio#1643, where Studio's row editor silently dropped a property deletion.

The workaround available today is delete then insert, which leaves the record absent between two writes (concurrent reads, relationship resolutions from the far side, and replicas all observe the gap), resets __createdtime__, and shows subscribers a delete followed by an insert instead of one write. put is Table.put — the same write REST performs, so the same audit type, replication shape, and retained created time.

update/upsert are deliberately untouched. 5da23c3ef originally used Table.put and hand-copied missing properties to emulate a merge; 5bd363946 replaced that with a real Table.patch, and b302a16e7 added the put fallback for a missing record. Merge semantics is load-bearing v4 compatibility, so this is a new operation rather than changed behaviour.

Two details worth review

The internal marker is an argument, not a request field. putRecords calls upsertRecords(obj, true). The first draft read full_record off the request, which let a client send full_record: true with an update and get a replace — making the operation name stop describing the write, and bypassing the attribute-scoped denial below, which is keyed on the operation. A test pins it; restoring the request read fails that test and nothing else.

A role with attribute_permissions on the table may not use put. Attribute permissions are checked against the attributes a request supplies (checkAttributePerms), which is enough for a merge but not for a replace, where an omitted attribute is removed — so such a role could otherwise erase an attribute it has no update permission for by leaving it out. REST closes the same gap in Table.allowUpdate by restoring those attributes from the stored record, which needs the stored record; authorization runs before anything is read, so it denies instead. Roles that scope no attribute are unaffected. Happy to swap this for a restore-based approach if you'd rather it mirror REST exactly.

Scope reduction

This replaces an earlier design (a full_record flag on update/upsert, plus a per-record __unset__ directive), per review. Three problems retire with it, none now reachable:

• __unset__ degraded a merge to a whole-record last-writer-wins put under contention — reachable single-node via the optimistic-lock retry, and unrecoverable in replication because the audit carried only the resolved put, not the deletion intent.
• The reserved __unset__ key reinterpreted a pre-existing user attribute of that name, with no migration.
• The directive interacted with dynamic attribute expansion, so a rejected write could still have registered attributes on an open table.

__unset__ is not abandoned — #2350 is now the feature request for doing it properly, with removal preserved as intent at the patch layer rather than resolved in the bridge. It's a genuinely better tool than put for "drop one field, don't resend the rest"; it just needs a reverse that carries the prior value, a defined removal-versus-set resolution, and a namespace that can't collide.

---

The authorization fix

Found while testing the attribute-permission denial; unrelated to the feature.

verifyPerms resolved the target database as requestJson.schema ?? requestJson.database. The handlers resolve it in commonUtils.transformReq, which runs after authorization. The two disagreed three ways, each letting a request be authorized against one target and executed against another.

Impact: any authenticated non-empty role had unconditional read/write/delete on the default data database, and could direct a write into any other named database. Not privilege widening — a role scoped exclusively to a second database, with zero permissions on data, read data.Dog in full (including an attribute it was explicitly denied), updated it, and deleted the record.

1. Neither key present. operationSchema was undefined, so schemaTableMap stayed empty; hasPermissions iterates that map, so it authorized by vacuous truth, and attribute permissions went unchecked too. Verified with a role holding update: false on data.Dog:

{"operation":"update","table":"Dog","records":[…]}                   → 200, record written
{"operation":"update","database":"data","table":"Dog","records":[…]} → 403 required_table_permissions:["update"]

Affected operations, all verified by running them: insert/update/upsert (write), delete, csv_data_load (async write — jobs authorize only at submit, so it rides into the worker), create_attribute (schema mutation), and all three search_* (read). describe_* and every requires_su operation fail closed. search_by_id with get_attributes:['*'] 500s rather than leaking.

2. database: 0, or any falsy-but-present value. ?? kept the 0, which is not a database, so the map stayed empty as in (1) — while transformReq, which tests falsy, defaulted to data and wrote there. Joi.number() is an accepted type, so 0 arrives validated.

3. Opposite precedence. verifyPerms preferred schema; transformReq prefers database. So {"schema":"data","database":"elsewhere"} was authorized against data and written to elsewhere — the worst of the three, since it reaches any named database.

Fix

One resolver, commonUtils.resolveTargetDatabase, which transformReq delegates to, so the two cannot drift again. Falsy rather than nullish, database over schema — matching what the handlers have always done, so no handler behaviour changes. transformReq writes only when the value would change: an unconditional assignment throws on a frozen object under strict mode.

Plus a fail-closed backstop in verifyPerms: a named table with an empty schemaTableMap is denied rather than authorized. Both that guard and the map population test != undefined, because hdbTable accepts a number and a truthy test dropped a table named 0 — the same vacuous-truth shape one guard over. getAttributePermissions too.

Also here, from review: the bulk-load attribute-permission opt-out is keyed on the operation being a known bulk load, never on a caller-supplied action. action is an unknown-but-accepted key on this validator, so inferring the opt-out from its presence let any direct request skip every attribute check by attaching it.

Affected versions

4.2.0 through 5.2.6 (current). transformReq's default arrived in 4.2.0 (5d5e58fc3); 4.1 declared schema required. The vacuous-map behaviour dates to ~4.0.8 (76ce6a2e3) but was inert until that default existed. release_4.7.19 has the identical expression at operation_authorization.js:350.

Not affected: REST, MQTT/WebSocket and GraphQL bind the database on the resource class at path-match time and never take it from the request body. The SQL path is the same bug class but a separate, already-fixed code path — and its fix is the stronger one, carrying the same per-reference backstop.

Regression risk

In-repo: nil. All 77 schema-less table-naming requests in integrationTests run as super_user, which returns before the target is used; all 491 non-SU requests already carry schema or database.

External: real, and release-note-worthy. A deployment whose clients send schema-less operations under a restricted role will start getting 403s. That is the vulnerability being closed, but it will look like a breaking change to anyone unknowingly relying on it.

Two intentional behaviour changes beyond the denials: structure_user: ['data'] + create_table with no database goes deny → allow (correct, previously untested), and search_by_id with get_attributes:['*'] and no database goes 500 → a proper allow/deny.

---

Testing

integrationTests/database/put-operation.test.ts — 18 probes, all passing on rocksdb.

put: the removal, update and upsert still merging (the controls), an explicit null staying a stored null, __createdtime__ surviving and __updatedtime__ re-stamping, create-and-replace, put_hashes reporting, parity with REST PUT (same stored attribute set through both doors), a relationship foreign key still resolving from the far side, a client-supplied full_record not turning an update into a replace, and the attribute-permission denial with its scope (the same role can still merge and insert).

Authorization: all three resolution divergences, asserted in both directions — a denial alone doesn't prove resolution, because the backstop also denies an unresolved target, so the role that does hold rights on the default database must still be allowed and its write must land there. A numeric table: 0. A caller-supplied action not skipping the attribute check.

Also pinned: on a legacy open table the operations-API attribute projection reports every registered attribute, filling in null for one the record doesn't have — so an absent attribute reads back as key: null, identically to one that was removed. A read-path artifact, not storage (verified by inserting a record that never had it), predating this work, and worth a test because it makes a successful removal look like a failed one.

Every guard mutation-checked — reverting it has to turn a test red:

mutation	result
put ignores the full-record marker	5 fail
marker read off the request (client-forgeable)	1 fail
attribute-scoped put denial dropped	1 fail
?? instead of || in the resolver	1 fail
schema before database	1 fail
no default at all	most fail
numeric-table guards reverted to truthy	1 fail
action-keyed bulk opt-out restored	1 fail

Also green: integrationTests/security/*.test.ts, apiTests/terminology.test.mjs (which deliberately exercises schema-less operations and asserts the data. default), database/{auto-fields,patch-disjoint-field-merge} — 143 with the put suite — plus 575 in northwind, which exercises the CSV and bulk-load paths this touches.

Cross-model review

Two passes via prepush-review.mjs, reviewers=codex+harper-domain, independent=true. Gemini failed on quota both times and both Cursor legs were skipped (their git fetch hits a local SSH-agent failure), so coverage is codex + domain only. Both passes predate this scope reduction — they reviewed the full_record + __unset__ design, and their findings are what drove it. Their still-live findings are fixed; the rest retired with the directive.

Not done

• Replication is untested here. put replicates as a put, the same as REST PUT already does, but there's no multi-node probe in this suite.
• LMDB run not done (HARPER_STORAGE_ENGINE=lmdb). The suite is written to run under it.
• Unit suites not run — a local Harper instance held the RocksDB lock throughout. Everything here is integration-covered; resolveTargetDatabase has no direct unit test.
• Docs not written. reference/operations-api/operations.md in the documentation repo needs a put entry, plus a note about the open-table read projection. Happy to open that PR once this shape is agreed.
• The put denial for attribute-scoped roles is blunt — it refuses rather than restoring unwritable attributes from the stored record the way REST does.
• Whether the security commit warrants a GHSA advisory, a CVE, and backports to 4.7/5.x is a maintainer call; the affected range above is what those would need.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces support for removing attributes over the operations API via the full_record: true flag (for full replaces) and the __unset__ directive (for merging while removing specific attributes). It also unifies target database resolution to resolve authorization bypass vulnerabilities and adds validation and permission checks for these new features. Feedback on the changes suggests making the takeUnsetAttributes helper function in ResourceBridge.ts self-safeguarding by adding defensive checks to verify that the unset parameter is a valid array before processing it.

[claude]

Reviewed; no blockers found.

[dawsontoth]

Blocker 1 (the merge → last-writer-wins put) is filed as #2350 (P2, Bug) rather than fixed here. The PR body carries the corrected claim, so the __unset__ contract as documented on this branch is single-writer.

#2350 lays out the three options: a removal operation in the resources/crdt.ts registry (which keeps the feature's purpose but needs a reverse that carries the prior value, plus a defined resolution for removal-racing-set), narrowing the documented contract to last-writer-wins like full_record, or restricting the directive to single-node. Option 2 is the honest short-term position if this merges before the design is settled.

🤖 Filed by Claude Code

[kriszyp]

Ok, I think this makes sense. However, would it make sense to add a put operation (upsert + full_record) instead? If we provided this, do we believe that update+full_record functionality and unset are actually meaningful gaps in functionality? Or do you have immediate use for these in the studio?
_{🤖 Reviewed with Codex}

[dawsontoth]

Yes — done in 1ac8d30. put is now the whole feature, and full_record and __unset__ are gone.

On your second question, the honest answer is that neither was a gap:

• Studio's need is exactly put. fix(queue): waitForDrain poll fallback for drains that never emit #1643 is the row editor: the user edits one record's JSON and saves, so the editor already holds the whole record. Nothing there wants a partial removal.
• update + full_record was only a safety nicety — "replace, but fail if the record is absent", so a mistyped primary key can't create a stray row. Studio can check that itself, having just loaded the record. Not worth a second spelling of the same write.
• __unset__ had a real justification — not resending the record — but it wasn't delivering it. Your stale-snapshot analysis is what settled that: resolved in the bridge it's a read-then-replace wearing a patch's clothing, and the audit carries only the resolved put, so the deletion intent can't survive replication. It needs to be a patch-layer operation or it isn't the thing it claims to be.

So it's filed as #2350 for doing properly (now a Feature, P3), with the reverse-value, removal-versus-set resolution, and namespace problems written up. Dawson wants it eventually — it's a genuinely nicer tool than put for "drop one field, leave the rest alone" — just not as a read-then-replace.

Three of your findings retire with the directive rather than being fixed: the stale snapshot, the reserved-key compatibility hazard (no key to reserve now), and the refusal-after-attribute-expansion ordering. The two that were about put's own surface are fixed and tested.

One thing your review caused that I'd flag: collapsing to a single operation exposed a bug in my own first draft of it. I initially had putRecords set full_record on the request object, which meant a client could send full_record: true with an update and get a replace — defeating the point of keying semantics on the operation name, and bypassing the attribute-scoped denial, which is keyed on the operation. It's an argument now, and there's a test that fails if it goes back on the request.

🤖 Reply from Claude Code