[jfrench9]

Summary

Hardening from a platform security review; companion to the robosystems bugfix/input-validation-hardening PR. Public text is deliberately brief.

• Block Explorer and Plan CSV exports neutralise formula-shaped text (one serializer, reusing textCell); numbers stay numeric
• The report viewer accepts ?graph= only as a graph id
• Approving a posting rule sends expected_updated_at (the row's updatedAt), so the approval covers exactly the version the person reviewed

Waits on

@robosystems/client minor with UpdateEventHandlerRequest.expected_updated_at (TS client PR on the same branch name). Typecheck is red until that publishes; the bump of every @robosystems/* package follows in this PR. No app release before the robosystems release is in prod — approvals of AI-suggested rules return 409 on the new API until this ships.

Test plan

• npm run test — 1232 passed (pre-commit)
• typecheck against the packed client branch

🤖 Generated with Claude Code

[github-actions]

Change classification: normal — platform or application code changed.

A normal change carries a documented automated review on this pull request before merge, in place of a second human approver. The change-classification job requests that review and passes once it has been posted. Recorded by the change-classification job.

[jfrench9]

@claude please review this PR

[claude]

Claude finished @jfrench9's task in 26s —— View job

---

Review of PR #527

The change is small and does what the description says. I found nothing blocking. Per the description, typecheck stays red until the client release lands, so I didn't run it.

CSV neutralisation (explorer/components/serialize.ts)

• Reusing textCell from lib/ledger/csv keeps one guard definition, and Plan now shares the Explorer serializer. The new tests cover formula labels and negative numbers.
• Adding \r to the quote trigger matches lib/ledger/csv.ts and is correct.
• The !Number.isNaN(Number(value)) check is a heuristic. Number() trims whitespace, so "\t1" or " " counts as numeric and skips the guard. "\t1" is harmless, but a label starting with a tab is exactly what textCell is meant to catch. A stricter check like /^-?\d+(\.\d+)?(e[+-]?\d+)?$/i would close that gap. Because csvField takes a string, the numeric/text distinction is also made after the fact. Passing number | string (as csvRow does) would be more robust than inferring it from text.
• planModel.ts now imports from explorer/components/serialize, which is a cross-route dependency. Per the colocation rule in CLAUDE.md, csvField would sit better in src/lib/ledger/csv.ts next to textCell and quote, with both routes importing from there. It's optional, and csvRow there already does most of this.
• Plan header cells and section.title also go through the guard. I think that's right, since titles are server-supplied.

Report viewer graph id (reports/[id]/content.tsx)

• The pattern ^[A-Za-z0-9_-]{1,128}$ is a sound allow-list. A bad ?graph= becomes null, and every call site already guards !graphId.
• reportId comes from params.id and is also interpolated into SDK request paths (getReportPackage, deleteReport, and so on). It isn't validated. Next decodes the route segment, so %2F or .. can arrive decoded. Applying the same check to it would complete the fix.
• The new constant is declared between the JSDoc for ReportViewerContent and the component, so the doc comment now sits above GRAPH_ID_PATTERN. Move the constant and helper above that comment.
• There's no test for safeGraphId. A case with ?graph=../x showing no request is made would lock it in.

Rule approval binding (posting-rules/content.tsx)

• Sending expected_updated_at: handler.updatedAt only when approve === true is the right scope. The tests assert it for both the plain approval and the priority-edit approval.
• Check what updatedAt is for rows created before updated_at existed. It's typed nullable in the test fixture (null was the old value). If it is null, the key is sent as null. Make sure the API treats that as "no check" rather than "must match null". If it should be omitted when absent, spread it conditionally.
• The API returns a 409 when the rule changed. Check whether friendlyError in src/lib/ledger/errors.ts maps that, so the user sees "this rule changed since you opened it, reload" and not a raw detail blob. load() isn't called on failure, so the row also stays stale. A refresh on 409 would help.
• The 409 for AI-suggested rules in the window before the app deploys is already called out in the description, so there's nothing to do beyond holding the release order.

Summary: good hardening. I'd fix the doc-comment placement and the reportId validation before merge. The 409 mapping and the null updatedAt case are worth confirming. The rest is optional.
· branch bugfix/input-validation-hardening

[jfrench9]

Companion PRs for the one release: robosystems RoboFinSystems/robosystems#1945 · robosystems-core RoboFinSystems/robosystems-core#98 · robosystems-typescript-client RoboFinSystems/robosystems-typescript-client#302 · robosystems-python-client RoboFinSystems/robosystems-python-client#286 · robosystems-app RoboFinSystems/robosystems-app#518 · roboledger-app #527