fix(roles): save attribute_permissions as [] so read-only roles can see their rows

[dawsontoth]

⊙ Problem

A user whose role grants read on a table could see the table in Browse but none of its rows (#1299). Since #894 (Nov 2025), the role editor has saved every table with "attribute_permissions": null whenever Pick Attributes is off, which is the default. Harper's role validation accepts and stores null. But verifyPerms reads .length from it whenever that role searches with get_attributes: ['*'], so the read fails with HTTP 500 There was an error when trying to choose an operation path (TypeError: Cannot read properties of null (reading 'length')). The grid's list query, the filtered list and the row editor all send ['*']. I reproduced this on Harper 4.7.40 and 5.3.1. The same role with [] reads normally and is still denied every other table.

❓ Your call: Should Studio fix this, or only Harper? I did both. Studio stops writing null here, and the server crash is filed as HarperFast/harper#3143 (P2), whose one-line guard would also repair every role already stored with null. Roles saved by earlier builds stay broken until someone re-saves them in the editor, or until #3143 ships. Studio could also work around those roles on the read side (see Alternatives).

💡 Solution

• The role editor's template writes "attribute_permissions": [] for an unpicked table instead of null.
• preparePermissionForSave sends any null it finds as []. That covers a role opened from storage, JSON pasted in, or a hand edit, so re-saving a broken role repairs it.

[] and null mean the same thing to Harper: no attribute scoping (getTableAttrPerms only scopes on a non-empty array). The only difference is that [] doesn't crash the '*' expansion.

⚖️ Alternatives

• Read-side workaround in Browse: send an explicit attribute list instead of '*' when the signed-in role's table entry is null. Explicit lists don't crash. This would fix already-stored roles with no admin action, but it changes get_attributes in three read paths (list, filtered list, search_by_id) plus export, and relationship and computed attributes behave differently under an explicit list. Not done; harper#3143 is the cleaner fix for stored roles.
• Omit the key: Harper rejects that (ATTR_PERMS_ARRAY_MISSING), so [] is the only safe value.

🔧 Changes

• defaultCalculator.ts: buildCurrent writes [] for the unpicked case.
• preparePermissionForSave.ts: the doc comment states the rule and links harper#3143. The non-elevated path (including a database-scoped structure_user array) now returns a copy with every table's null replaced by []. The copy is rebuilt with Object.fromEntries, so a database or table named __proto__ stays an own key. Only an exact null is rewritten: scoped attribute_permissions and legacy attribute_restrictions pass through untouched, and the input is not mutated. The elevated-role path is unchanged, since it drops table permissions anyway.
• usePermissions.ts: one comment that said defaultCalculator writes null now says earlier builds saved it. Readers still accept null, because stored roles carry it.

✅ Verification

• Live reproduction (end-to-end route): I used two throwaway local servers, Harper 4.7.40 (docker) and 5.3.1 (the harper checkout's dist, isolated HOME and ROOTPATH). On each I created a role the way Studio does (add_role, then alter_role with the editor document, read: true on one table) plus a user in it, and replayed Browse's exact request bodies as that user. With null: describe_all and describe_table returned 200, while search_by_value, search_by_conditions and search_by_id with ['*'] all returned 500. I then built the alter_role payload with the fixed calculateDefaultPermissions + preparePermissionForSave from the server's real describe_all, starting from the stored null role, and applied it. After that the list and search_by_id returned the rows on both versions, the other table still returned 403, and insert returned 403.
• Tests: defaultCalculator.test.ts checks that the template writes [] both for a fresh role and for one stored with null. preparePermissionForSave.test.ts covers null becoming [] without mutating the input, scoped and legacy entries left untouched, the database-scoped structure_user path and __proto__ names. Its shared fixture moves to [] so the "untouched" cases still mean untouched. EditRoleModal.test.tsx drives the real modal: saving a new read-only role, and re-saving one stored with null, both send [] through alter_role.
• Fails on base: with both source files reverted to origin/stage, 6 tests fail, and the modal test's diff is exactly "attribute_permissions": null. Reverting each file alone turns its own tests red (1 and 4). Swapping Object.fromEntries for plain assignment fails the __proto__ test.
• Gates: the full vitest run passed (395 files, 3670 tests). tsc -b, oxlint and dprint check all exited 0. The pre-commit hook passed without --no-verify.
• Cross-model review: two full rounds, codex and gemini on both, so two outside families. Neither raised a correctness finding. The Cursor legs could not fetch through the 1Password SSH agent, and the Harper domain (adjudication) leg's OAuth session had expired, so outside findings were triaged by hand rather than adjudicated. I rejected Gemini's "a null or array JSON root crashes or reshapes the save": the only caller already requires a plain object, at EditRoleModal.tsx.

Closes #1299

🤖 Generated with Claude Code

_{🤖 Generated by Anthropic Claude (Opus 5.5); posted via @dawsontoth.}

Related PRs: none found
Complexity: easy

_{Review-Coverage: authored=claude; ran=codex,gemini; blocked=cursor-composer(no-receipt),domain(auth),cursor-muse(no-receipt); declined=cursor-grok,cursor-kimi; rounds=2; full=1 @ d878005}

_{Review-Attention: read ~3m (raised: degraded review) @ d878005}

[gemini-code-assist]

Code Review

This pull request ensures that attribute_permissions is saved as an empty array [] instead of null to prevent 500 errors in HarperDB. The changes update defaultCalculator.ts and preparePermissionForSave.ts to convert null values to [], with careful handling of potential prototype pollution (e.g., __proto__ keys). Comprehensive tests have been added to verify these fixes across different scenarios. I have no additional feedback to provide as there are no review comments.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	67.54%	10106 / 14962
🔵	Statements	67.7%	10784 / 15929
🔵	Functions	60.81%	2603 / 4280
🔵	Branches	62.64%	7708 / 12305

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/features/instance/config/roles/defaultCalculator.ts	75.75%	38.46%	55.55%	74.19%	65-69, 88-122, 142-161
src/features/instance/config/roles/preparePermissionForSave.ts	94.73%	96.55%	100%	94.73%	60
src/hooks/usePermissions.ts	59.13%	55.78%	71.42%	59.29%	59-65, 86-93, 126, 131, 202, 205, 208, 220-229, 267-269, 287-289, 311-350

Generated in workflow #2095 for commit d878005 by the Vitest Coverage Report Action