Guest web-to-case submissions can plant customer_rating and customer_feedback

Found while working #1428 (surfacing crm_case.internal_notes). Not fixed there — src/objects/case.hook.ts is outside that card's file surface.

The measurement

An anonymous insert through the real engine (memory driver, SecurityPlugin + SharingServicePlugin mounted, the same harness test/guest-submission-sanitisation.test.ts uses) planting five keys:

subject, description, internal_notes: 'PLANTED-NOTES',
customer_rating: 5, customer_feedback: 'PLANTED-FEEDBACK'

stores:

{
  "origin": "web",
  "internal_notes": null,
  "customer_rating": 5,
  "customer_feedback": "PLANTED-FEEDBACK"
}

origin: "web" is the positive control this repo already uses: crm_case.origin declares no defaultValue, so that value can only have come from the first statement of the guest branch itself. The branch ran, nulled internal_notes, and left both survey fields exactly as the submitter sent them.

Why it is a hole rather than a second layer

case.hook.ts says so itself, in the comment above the branch:

guest_portal grants crm_case.allowCreate at the OBJECT level with no field allow-list, which is precisely why this branch is the field-level control and why an omission from it is a hole rather than a second layer.

The branch nulls owner_id, is_escalated, escalation_reason, internal_notes and resolution. customer_rating and customer_feedback are not in it. Field-level permissions cannot cover them either: @objectstack/plugin-security's getFieldPermissions builds its mask ONLY from fields a profile names in its fields map, and no profile in src/profiles/ names either one — so absence means unrestricted, not restricted.

The public web_to_case form collects only subject / description / type / priority, so this is not reachable through the shipped form. It is reachable through the same API surface the form posts to, which is the surface the hook branch exists to police.

Why it matters beyond tidiness

customer_rating is the satisfaction score. case_csat_followup exists to collect it, and case_metrics reporting reads case data — so the one field an outsider can self-report is the one the CRM would report on as a quality measure. A submitter can open a case and give themselves five stars in the same request.

Where the fix goes

The two assignments belong in the same isGuestSubmission branch, spelled as assignments (= null) rather than delete, for the reason the branch header documents at length. test/guest-submission-sanitisation.test.ts is where the pin belongs — its first case plants the internal fields and asserts each is blanked; these two names simply need to join that list.

⚠️ Adjacent but separate: whether these fields should have a staff-facing authoring surface at all is the open product question on #1428. This card is only about the intake path and does not depend on that ruling either way — a field nobody may author from the UI certainly should not be authorable by an anonymous submitter.

Generated by Claude Code

First-touch grading by the repo:hotcrm seat (R26): pm:queue + bug + metadata, type Bug.

Bug on the strongest possible ground — the repo's own code declares the contract this violates. Verified verbatim at src/objects/case.hook.ts:131-132:

guest_portal grants crm_case.allowCreate at the OBJECT level with no field
allow-list, which is precisely why this branch is the field-level control
and why an omission from it is a hole rather than a second layer.

⇒ The branch is the field-level control by declaration, so an omission from it is a hole by the app's own stated design, not by this seat's opinion. ⚠️ Checked with two short fragments read separately rather than one long pattern — a line-oriented grep cannot match a phrase that wraps, and that mistake cost this lane a false accusation in R25.

Independently re-measured

⛔ Not taken on the reporter's word:

isGuestSubmission = !ctx.previous && !ctx.user?.id && !ctx.session?.isSystem (:145) ✅ — the anonymous intake path, exactly as described.
The branch nulls five fields — owner_id, is_escalated, escalation_reason, internal_notes, resolution (:159-163) ✅.
customer_rating and customer_feedback never appear anywhere in case.hook.ts ✅. Reverse control for that zero: five sibling crm_case field names do appear in the same file, so the pattern class matches there and these two are genuinely absent rather than merely unmatched.

⭐ A qualification the card does not carry — and it bounds the severity

Six lines above the quote it cites, the same comment block says:

⚠️ What is measured here is the HOOK-LEVEL control. A real anonymous write also
crosses plugin-security's middleware, and nobody has measured that path — so
this is coherence and defence in depth, NOT a claim that a guest reaches the
column in production.

The card's measurement is an insert through the engine harness. That is a real measurement of the engine path, and it is not the same thing as an HTTP request through the REST middleware — which is the path the previous author explicitly flagged as unmeasured. ⇒ "A submitter can open a case and give themselves five stars" is established for the engine path and NOT established for production.

⛔ The dispatched dev must not carry that sentence into a PR as an established fact. Two consequences:

The fix is correct regardless and does not depend on the answer — a field omitted from the branch that is declared to be the field-level control should be in it, whether or not a second layer happens to catch it today. That is why this stays pm:queue and not a decision card.
The severity claim needs the REST path measured before anyone writes it down. Measuring it is in scope and is the more valuable half; if it turns out the middleware does block it, say so — a defence-in-depth fix with an honest "the outer layer already held" is a better record than an unqualified breach claim.

⚠️ Also reported, not confirmed by this seat: the claim that @objectstack/plugin-security's getFieldPermissions builds its mask only from fields a profile names, so absence means unrestricted. This checkout has no node_modules, so this seat cannot read the installed platform. ⛔ The dev re-measures it against the installed tree; it is load-bearing for "field-level permissions cannot cover them either".

No priority label

Deliberate. It is not prio:p1 — the shipped web_to_case form collects only subject / description / type / priority, and customer_rating currently has no read surface anywhere (no section, no highlight, no list column, no filter), so nothing displays the forged value today. It is also not backlog: the fix is two assignments plus a test pin in a file that already has the matching case, and the field is about to acquire meaning. Worth taking early on cost, not on alarm.

Correctly filed, and correctly independent

⭐ Filing this instead of riding it into PR #1506 was right — src/objects/case.hook.ts is outside #1428's declared surface. And the card is right that it does not depend on #1428's product ruling: a field nobody may author from the UI certainly should not be authorable by an anonymous submitter. ⇒ It is dispatchable now, ⛔ not blocked behind #1428.

Generated by Claude Code

Claim: session_018Z7RPTnPnXsb8RUBr4yDjE · repo:hotcrm seat · R27 · branch claude/issue-1505-guest-survey-fields · dedicated worktree · tier opus (standing exemption, maintainer 2026-08-13: 「fable 如果用完了,可以用 opus」, floor opus).

Declared file surface: src/objects/case.hook.ts · test/guest-submission-sanitisation.test.ts. ⚠️ Derive dependents by grep before editing and read every hit — case.hook.ts is imported by several runtime-hook suites.
Serial constraints: none live. ⛔ R27's other two cards are docs/ and content/docs/ — no overlap. ⚠️ #1428 is needs-user-decision and is not a blocker: this card is independent of that ruling by construction.
Clause-② in full: falsify rather than implement past a false premise.

Zone 1 — Rulings

None.

Zone 2 — Re-measured by this seat on `main` @ `39aae451` (⛔ verify anyway)

isGuestSubmission = !ctx.previous && !ctx.user?.id && !ctx.session?.isSystem ✅
The branch nulls exactly five: owner_id, is_escalated, escalation_reason, internal_notes, resolution (:159-163) ✅
customer_rating / customer_feedback appear nowhere in that file ✅ — reverse control: five sibling field names do appear there, so the pattern class matches and these are genuinely absent.
The contract is stated in the file itself, verified verbatim at :131-132: "…which is precisely why this branch is the field-level control and why an omission from it is a hole rather than a second layer."

⛔ Zone 2's most important item: the severity claim is NOT established

Six lines above that quote, the same comment block says:

⚠️ What is measured here is the HOOK-LEVEL control. A real anonymous write also crosses plugin-security's middleware, and nobody has measured that path — so this is coherence and defence in depth, NOT a claim that a guest reaches the column in production.

The reporting probe measured the engine path (a harness insert). That is not the REST path an actual anonymous submission takes. ⛔ Do not carry "an anonymous submitter can give themselves five stars" into the PR as established fact.

⇒ Measuring the REST/middleware path is the higher-value half of this card. Either outcome is a good result: if the middleware already blocks it, say so plainly and the fix becomes defence-in-depth with an honest record; if it does not, you have established a real unauthenticated integrity hole. ⛔ What is not acceptable is shipping the fix while leaving the severity implied.

⚠️ Also reported, not confirmed by this seat: that plugin-security's getFieldPermissions builds its mask only from fields a profile names, so absence means unrestricted. This checkout has no node_modules. Re-measure it against the installed tree — it is load-bearing.

Zone 3 — Suggested

The two assignments belong in the same isGuestSubmission branch, spelled = null rather than delete, for the reason the branch header documents. test/guest-submission-sanitisation.test.ts already has the matching case — its first test plants the internal fields and asserts each is blanked; these two names join that list.

⛔ Do not widen the branch beyond these two. If you find a third omitted field, report it — ⛔ do not fold it in silently.

Constraints

⛔ Locate by text, not line number.
⛔ Never skip, disable or loosen an assertion.
⛔ No content/docs/releases/ edits.
Changeset required. Draft PR, Fixes #1505 — ⚠️ only if the card is fully addressed; if the middleware measurement splits it, use Part of and say so.

Generated by Claude Code

os-dev-report

{
  "issue": 1505,
  "status": "done",
  "branch": "claude/issue-1505-guest-survey-fields",
  "pr": "https://github.com/objectstack-ai/hotcrm/pull/1515",
  "premise_still_valid": true,
  "summary": "The two assignments (`input.customer_rating = null`, `input.customer_feedback = null`) join the isGuestSubmission branch in src/objects/case.hook.ts, and test/guest-submission-sanitisation.test.ts pins them in both directions — the guest case plants and asserts blanked, the trusted-write negative control plants and asserts they SURVIVE (case_csat_followup notifies the case owner to log a rating, so the strip must stay guest-scoped). The branch was not widened beyond these two; no third omitted field exists. THE SEVERITY HALF: the REST/middleware path is now measured against a real booted server, and it BLOCKS the write — every generic anonymous surface answers 401 (data insert, :id update, batch, actions, mcp), and the single anonymous write path that survives (POST /api/v1/forms/support/submit) filters the body against an allow-list built from the matched form view's own declared sections, so a planted customer_rating never reaches ObjectQL. A guest does NOT reach these columns in production. One sentence of the issue body is therefore FALSIFIED — 'It is reachable through the same API surface the form posts to' is not true of the shipped app — while the premise the fix rests on (the branch is declared to BE the field-level control, and omits two fields) stands, measured. The fix is honest defence in depth, and the PR says so in those words rather than implying a breach. What makes it load-bearing rather than cosmetic is also measured: the allow-list is the form's FIELD LIST, a product decision, so adding customer_rating to web_to_case's sections — one line, the exact edit #1428's open question would make — stored a planted customer_rating: 5 on the pre-fix hook, and null on the post-fix hook.",
  "tests": "All on final commit 796f4ca1. (1) FULL CHAIN: `pnpm verify` (validate && typecheck && lint && lint:i18n-gate && hygiene && hygiene:tokens && build && test) under the shared verify lock — `os-verify-lock: VERDICT command-exit 0`; its own printed verdicts: `✓ Validation passed`, `✓ i18n lint gate: 0 i18n/missing-* issues`, `✓ source hygiene clean` (incl. `no raw control bytes in first-party files`), `✓ source token ratchet clean` (business semantics ~83,079 of ceiling ~85,000), `✓ Build complete`, `Test Files 156 passed (156)`, `Tests 3294 passed | 1 skipped (3295)`. `changeset status --since=origin/main` → hotcrm patch. (2) REVERSE VERIFICATION, direction predicted RED and observed RED: fix committed FIRST, then only the two new assignments ablated; on-disk confirmed by grep count 0 for the removed line plus a non-empty `git diff --stat`; vitest exit 1 with `AssertionError: a guest-planted satisfaction rating survived the strip … expected 5 to be null`, `Tests 1 failed | 5 passed (6)`; restore proved by comparing `git hash-object` against the `HEAD:` blob (c60900e2), not by an exit code, under an `EXIT INT TERM` trap using absolute paths. (3) REST MEASUREMENT on a real server (`objectstack start`, plugins: Auth, Security, Sharing, RestAPI, hotcrm; sqlite): anonymous POST /api/v1/data/crm_case 401, PATCH /api/v1/data/crm_case/:id 401, POST /api/v1/data/crm_case/batch 401, POST /api/v1/actions/crm_case/create_case 401, POST /api/v1/mcp 401, POST /api/v1/forms/support/submit 201 storing customer_rating=null, customer_feedback=null with origin='web' as the positive control that the guest branch ran. Allow-list positive control in ONE request: planted priority='critical' and type='problem' (both declared on the form) LANDED — priority_rank 4, 4h SLA — while the two undeclared survey fields came back null, which identifies the form's declared-section allow-list as the mechanism rather than a blanket reject. GET /api/v1/forms/support returns an objectSchema of exactly 4 fields; customer_rating is not in it. (4) COUPLING PROBE, mutation trapped and proved on disk (marker grep count 1) and in the built artifact (`ARTIFACT web_to_case fields: [subject, description, type, priority, customer_rating]`), source restored byte-identical to HEAD (14f5902b) both times: widened form + PRE-fix hook stored customer_rating=5; widened form + POST-fix hook stored null; customer_feedback, left off the widened form, stayed null in both, which is the negative control. (5) INSTALLED-TREE re-measurement of the load-bearing claim, read from node_modules/@objectstack/plugin-security 17.2.0 dist: `getFieldPermissions` iterates `ps.fields` and skips any key not prefixed `${objectName}.`, so an unnamed field is simply absent from the mask; `getNonEditableFields` maps only entries present in the mask; `detectForbiddenWrites` returns [] when the mask is empty. src/profiles/guest-portal.profile.ts declares no `fields` key at all ⇒ empty mask ⇒ the write guard is skipped outright. Absence means UNRESTRICTED, confirmed. All three probe servers were torn down by exact PID (the CLI parent spawns a child that outlives it — killed the listeners found via lsof on my own ports 4051/4052/4053, never by process name); ports confirmed closed.",
  "mcp_calls": "9",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #1516 (finding, unassigned): test/case-guest-branch-leftovers.test.ts still carries the sentence this PR retired from case.hook.ts — that a real anonymous write crosses plugin-security's middleware and 'nobody has measured that path'. Now false, and it instructs future readers how to read a failure of the cases below it, so it is not merely stale prose. Outside this card's declared file surface, so reported rather than folded in."
  ]
}

Generated by Claude Code

refactor(case): retire customer_rating, customer_feedback and the CSA…

Navigation Menu

Guest web-to-case submissions can plant customer_rating and customer_feedback #1505

The measurement

Why it is a hole rather than a second layer

Why it matters beyond tidiness

Where the fix goes

Independently re-measured

⭐ A qualification the card does not carry — and it bounds the severity

No priority label

Correctly filed, and correctly independent

Zone 1 — Rulings

Zone 2 — Re-measured by this seat on `main` @ `39aae451` (⛔ verify anyway)

⛔ Zone 2's most important item: the severity claim is NOT established

Zone 3 — Suggested

Constraints

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Guest web-to-case submissions can plant customer_rating and customer_feedback #1505

Description

The measurement

Why it is a hole rather than a second layer

Why it matters beyond tidiness

Where the fix goes

Activity

hotlong commented on Sep 3, 2026

Independently re-measured

⭐ A qualification the card does not carry — and it bounds the severity

No priority label

Correctly filed, and correctly independent

hotlong commented on Sep 3, 2026

Zone 1 — Rulings

Zone 2 — Re-measured by this seat on main @ 39aae451 (⛔ verify anyway)

⛔ Zone 2's most important item: the severity claim is NOT established

Zone 3 — Suggested

Constraints

hotlong commented on Sep 3, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Zone 2 — Re-measured by this seat on `main` @ `39aae451` (⛔ verify anyway)