diff --git a/CLAUDE.md b/CLAUDE.md index fbde703..1afa727 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1376,7 +1376,14 @@ Rendered in `dashboard.html` for `current_user.role == 'customer'`. Uses a Boots - Each bubble shows: colored avatar circle (color keyed to `author.id % 7`), display name, role badge (Staff / Customer), `is_customer_visible` badge (staff-only view), status-at-time badge, timestamp, and body. - **Staff commenting:** A hidden checkbox `name="is_customer_visible"` in the Add Comment form defaults to unchecked (staff-only). Checking it marks the comment visible to customers. - **Customer commenting:** Only shown when `can_customer_comment = is_following or issue.reported_by == current_user.id`. Customer POST bypasses `IssueUpdateForm`; the route sets `is_customer_visible=True` unconditionally. -- **Read filtering:** `GET issues/view` passes `filter_by(is_customer_visible=True)` to customers; staff receive all comments. **Gated by `COMMENTS_VISIBLE_TO_ALL` (temporary, Aug 2026)** — while that config is true the filter is skipped entirely and customers see every comment. The route passes `comments_open` to the template, which then (a) suppresses the per-comment "Customer visible" / "Staff only" badges, since they would misstate what the customer can actually see, and (b) hides the "Share with customer" tick behind a warning banner reading *"Comments are currently visible to everyone… Do not post internal-only notes here."* The checkbox value is still posted and stored, so flipping the config back restores both the filtering and the badges immediately. +- **Read filtering:** `GET issues/view` passes `filter_by(is_customer_visible=True)` to customers; staff receive all comments. **Gated by `COMMENTS_VISIBLE_TO_ALL` (temporary, Aug 2026)** — while that config is true the filter is skipped entirely and customers see every comment. The route passes `comments_open` to the template, which then (a) suppresses the per-comment "Customer visible" / "Staff only" badges, since they would misstate what the customer can actually see, and (b) hides the "Share with customer" tick behind a warning banner reading *"Comments are currently visible to everyone… Do not post internal-only notes here."* + +**Both are gated on `viewer_is_our_staff` (Aug 2026), not on `role != 'customer'`.** `can_edit` is true for a **Customer Inspector assigned to the issue**, so that account was being shown the staff comment form complete with an internal-process warning and the internal visibility badges. Neither means anything to a customer — nothing they write was ever private — and both expose how we work. The comment box itself is unaffected; only the internal chrome is hidden. + +`viewer_is_our_staff` is one `{% set %}` at the top of the template, written as an explicit **allowlist** — `current_user.role in ['admin','director','project_manager','auditor','inspector']` — for two reasons: + +- **It fails closed.** The obvious form, `not current_user.is_customer_account`, fails **open**: if that attribute is missing for any reason — most realistically a process still running an older `models/user.py` while templates have already reloaded — Jinja yields `Undefined`, `not Undefined` is true, and the internal text renders for exactly the accounts it must be hidden from. This was observed in practice. An allowlist of literal role strings can only be true for a role actually listed. +- **`external_inspector` is absent on purpose, and this is NOT a rule-87 violation.** Rule 87 governs capability and scoping checks, where a Customer Inspector must behave exactly like our own inspector. This asks a different question — *does this person work for us?* — which is the one place the two roles genuinely differ. Do not add `external_inspector` to this list. The checkbox value is still posted and stored, so flipping the config back restores both the filtering and the badges immediately. ### List filter preservation (Aug 2026) diff --git a/app/templates/issues/view.html b/app/templates/issues/view.html index 55452cf..9462253 100644 --- a/app/templates/issues/view.html +++ b/app/templates/issues/view.html @@ -23,6 +23,26 @@ preservation). Threaded into every action so an update or delete returns to the same filtered page, and used by the Back button. #} {% set back_url = request.args.get('next') or url_for('issues.index') %} +{# Is the viewer OUR staff? Drives the internal-only chrome on this page: the + "comments are visible to everyone" warning and the per-comment + "Customer visible" / "Staff only" badges. Both are instructions about how WE + work and must never reach a customer account. + + Written as an explicit ALLOWLIST of our own roles, deliberately: + + * It FAILS CLOSED. The obvious form, `not current_user.is_customer_account`, + fails OPEN — if the attribute is missing for any reason (a process still + running an older models/user.py after a template-only reload, say) Jinja + yields Undefined, `not Undefined` is true, and the internal text is shown + to exactly the people it must be hidden from. An allowlist of literal role + strings can only ever be true for a role we listed. + * `external_inspector` is absent ON PURPOSE. This is NOT the rule-87 case: + rule 87 is about capability/scoping, where a Customer Inspector must + behave exactly like our own inspector. Here the question is "does this + person work for us?", which is the one place the two genuinely differ. + Do not "fix" this by adding external_inspector to the list. #} +{% set viewer_is_our_staff = current_user.role in + ['admin', 'director', 'project_manager', 'auditor', 'inspector'] %}
{# ══════════════════════════════════ LEFT COLUMN ══════════════════════════════════ #} @@ -244,7 +264,7 @@ While comments_open is set, EVERY comment is visible to the customer, so the old "Staff only" badge would be a lie. It is suppressed rather than shown incorrectly. #} - {% if current_user.role != 'customer' and not comments_open %} + {% if viewer_is_our_staff and not comments_open %} {% if c.is_customer_visible %} @@ -299,8 +319,14 @@ the "Share with customer" tick decides nothing. Saying so plainly matters: a staff member must not write something they believe is private. The checkbox is still posted and recorded, so turning the - setting off restores its meaning immediately. #} - {% if comments_open %} + setting off restores its meaning immediately. + + OUR STAFF ONLY. `can_edit` is also true for a Customer Inspector + assigned to the issue, and this banner is an internal-process + warning ("do not post internal-only notes") — showing it to a + customer account exposes how we work and reads as nonsense to + them, since nothing they write was ever private. #} + {% if comments_open and viewer_is_our_staff %}
Comments are currently visible to everyone, including