Skip to content

fix(access-policy): preserve user-supplied id in the policy builder - #122

Merged
Panthevm merged 2 commits into
masterfrom
fix/8074-accesspolicy-builder-preserve-id
Jul 28, 2026
Merged

fix(access-policy): preserve user-supplied id in the policy builder#122
Panthevm merged 2 commits into
masterfrom
fix/8074-accesspolicy-builder-preserve-id

Conversation

@aleksandrkislitsyn

Copy link
Copy Markdown
Contributor

Fixes HealthSamurai/sansara#8074.

Problem

Creating an AccessPolicy in the resource browser with an explicit id (e.g. id: test-id) produced a resource with a different, server-assigned id.

Root cause

The AccessPolicy builder (builder-content.tsx) stripped id/resourceType/meta out of the editable text and kept them in separate state. On every keystroke, handleTextChange re-merged the parsed text with id: accessPolicy.id.

On the create page the initial resource has no id, so accessPolicy.id is undefined. The typed id was therefore overwritten with undefined, and the create request went out with no id — so the server assigned a random one. (Confirmed against a running Aidbox: a create request that does carry a body id honors it; the UI was sending none.)

This was specific to the AccessPolicy custom builder — the generic resource editor sends the full edited text and was already correct.

Fix

Stop stripping id. It now stays visible and editable in the editor, and flows from the parsed text into the saved resource:

  • Create: typing id: test-id keeps the id in the editor and creates the policy with test-id.
  • Edit: existing policies show their id; the update still uses the route id for the URL, so the id cannot be changed by editing.

resourceType (fixed for AccessPolicy) and meta (server-managed) remain stripped.

Testing

  • pnpm biome check — clean.
  • pnpm typecheck — no new errors (two pre-existing notebook-chart/fold-data.ts codemirror errors on master are unrelated).
  • Manually verified in the dev server: create an AccessPolicy with id: test-id → id stays visible in the YAML and the resource is saved with that id.

🤖 Generated with Claude Code

Creating an AccessPolicy with an explicit `id` in the builder produced a
resource with a different, server-assigned id (issue #8074).

The AccessPolicy builder stripped `id` out of the editable text and, on every
edit, re-merged the parsed text with `id: accessPolicy.id`. On the create page
`accessPolicy.id` is undefined, so a typed id was overwritten with undefined
and the create request carried no id — the server then assigned a random one.

Fix: stop stripping `id`. It now stays visible and editable in the editor and
flows from the parsed text into the saved resource, so a typed id is honored on
create and shown on edit. `resourceType` (fixed) and `meta` (server-managed)
remain stripped.

Co-Authored-By: Aleksandr Kislitsyn <aleksandr.kislitsyn@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aleksandrkislitsyn

Copy link
Copy Markdown
Contributor Author

Note on the failing audit check

The audit job failure is pre-existing and unrelated to this PR. CI runs pnpm audit --audit-level=high, which queries the live advisory database at run time. It's flagging transitive dependencies only:

This PR changes a single component (builder-content.tsx) and touches no dependencies or lockfile (git diff master... shows only that one file). Earlier master/PR runs passed only because they ran before these advisories were published — same lockfile, newly flagged.

build, lint, and typecheck all pass. The dependency advisories should be addressed by a separate lockfile-bump PR (there's already an audit-fixes branch), not folded into this fix.

CI `pnpm audit --audit-level=high` flagged three transitive/direct advisories
present in the lockfile:

- postcss  GHSA-r28c-9q8g-f849  -> override floor to >=8.5.18
- seroval  GHSA-mv8w-475r-vwqw  -> override to >=1.5.3 (via @tanstack/react-router)
- js-yaml  GHSA-pm4m-ph32-ghv5  -> pin <5.2.2 to 5.2.2

Also consolidates the three conflicting js-yaml overrides (which were forcing
the direct `^4.1.1` dep up to a vulnerable 5.2.1) into a single one targeting
the patched 5.2.2. Only .load()/.dump() are used, so the 5.2.1 -> 5.2.2 patch
bump carries no API risk.

Co-Authored-By: Aleksandr Kislitsyn <aleksandr.kislitsyn@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aleksandrkislitsyn

Copy link
Copy Markdown
Contributor Author

Update: pushed 8638ec7 to fix the audit check in this PR.

These advisories were newly published against deps already in the lockfile (not introduced here). Since no existing commit fixed them, I added pnpm overrides:

Also consolidated three conflicting js-yaml overrides (which were forcing the direct ^4.1.1 dep up to a vulnerable 5.2.1) into one targeting 5.2.2. Only .load()/.dump() are used, so the 5.2.1 → 5.2.2 patch bump carries no API risk.

pnpm audit --audit-level=high now reports no known vulnerabilities; typecheck and lint remain green.

@Panthevm
Panthevm merged commit ad1fbb6 into master Jul 28, 2026
4 checks passed
@aleksandrkislitsyn
aleksandrkislitsyn deleted the fix/8074-accesspolicy-builder-preserve-id branch July 28, 2026 08:12
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