Skip to content

fix(attachments, demo-staff, email docs): reps list the attachments of records they can read; demo:staff runs; the email page names the shipped templates (#2029 items 1, 2, 4) - #2036

Merged
objectstack-fleet[bot] merged 8 commits into
mainfrom
claude/issue-2029-items-1-2-4
Oct 9, 2026

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Part of #2029
Clause-②: no

Items 1, 2 and 4 of #2029. Item 3 (the Quotes page's nightly-expiry claim) is not addressed here: it edits content/docs/sales/quotes*.mdx, which #2014 and #2032 held, and it follows them.

Measured before the fix

On fresh objectstack dev boxes on port 4934 at base f071b36c (@objectstack/* 17.7.0), with the demo personas created by pnpm demo:staff.

Item 2: pnpm demo:staff exits 1 at step 3.

  • pnpm demo:staff --url http://localhost:4934 exited 1 with POST /api/v1/data/crm_account/query → 400: Invalid query request. The server's envelope names the field: {"field":"query.filters","code":"min_items"}.
  • With only the query shape fixed, the same step failed again on that box: PATCH crm_opportunity/DnwI-VvYuiQYOlOU → 409: Opportunity 'Acme Platform Upgrade' is locked while an approval is in progress. The seed prices exactly two open deals over the large-deal line on purpose (Acme Platform Upgrade, Wayne Enterprise License, [finding] Seed data puts every ≥$100K opportunity into a pending approval — ~90% of open kanban cards are undraggable in a fresh demo database #1902). On that box the line-item amount rollup updated both after the record-change trigger was armed, so both opened opportunity_approval with lockRecord: true. The platform then refuses every edit to them, the dev admin's ownership PATCH included. On a second fresh boot the two approvals had not opened before the script ran, so whether a box has them depends on seed timing.

Item 1: a rep is refused the quote's attachments.

  • As na.rep (sales_rep + na_sales_team), GET /api/v1/data/sys_attachment answered 403 PERMISSION_DENIED. /api/v1/auth/me/permissions showed "permissionSets":["sales_rep","member_default"] and no sys_attachment entry. An upload to a quote the rep can read got the same 403. eu.rep, sales.manager, service.agent and service.manager all got the same 403. The admin listed both test attachments.
  • The producer is the app's permission sets. The platform's member_default (the everyone baseline) names sys_attachment nowhere. Since it was narrowed to explicit-allow (member_default's * wildcard object grant (C/R/E) union-merges into every org member — app-side explicit-allow object gates are erased on three axes objectstack#5491), the rule is "Everything else is the application's to declare". The platform's attachments-access page says that attachment management "is enabled by an ordinary, position-distributed permission set that grants sys_attachment CRUD". No set in this app granted it, and the enable.files note said none had to.

Item 4: the Email & Calendar page says "this app authors no templates". As admin, GET /api/v1/data/sys_email_template served 100 rows. 76 are crm.*: 19 names, each in en-US, zh-CN, ja-JP and es-ES, category: workflow, managed_by: package. The other 24 are the platform's auth.* rows. Every crm.* name is the template: of a notify node in this app's flows.

The fix, and why it is the minimal one

Item 2, scripts/demo-staff.ts.

Item 1, src/sales/profiles/*.profile.ts and the enable.files note in src/sales/objects/index.ts.

  • Six sets can read a files-enabled object: sales_rep, sales_manager, marketing_user, service_agent (and service_manager, which spreads it), and system_admin (and tenant_admin, which spreads it). Each now grants sys_attachment with allowRead: true and every other bit false. guest_portal grants nothing.
  • Which rows that read reaches stays with the platform's parent-derived filter in service-storage.
  • Measured after on a fresh box (HEAD ee14b076): na.rep lists the attachment on the Acme quote but not the one on a Northwind (EMEA) quote. eu.rep sees the reverse, and sales.manager sees both. na.rep downloads the first file (200) and is refused the second (403 ATTACHMENT_DOWNLOAD_DENIED). In the console, the quote's Attachments tab lists the file.

Why read only, and not read, upload and delete. The first version of this branch granted all three bits, and measuring it found a platform defect. The attach and delete gate asks the sharing service canEdit(parent). plugin-sharing's effectiveSharingModel maps controlled_by_parent to public, so canEdit answers true for every quote, contract and contact. With the write bits granted, na.rep:

  • attached a file to a Northwind quote that answers them 404: 201;
  • attached a file to an Acme contract whose own PATCH answers them 403 (201), and deleted the admin's executed-contract file from it (200);
  • was refused the same attach to the private Northwind account: 403 ATTACHMENT_PARENT_ACCESS.

That is wider than "the attachments of the records they can read, no wider". So this branch withholds the write bits and does not build a workaround (AGENTS.md §2). The defect goes upstream through the seat. Until its fix is in the pinned version, only administrators upload. The canonical note and the structural test both say so.

Item 4, content/docs/guides/email-and-calendar*.mdx (en, zh-Hans, zh-Hant). The false sentence is replaced. The page now says:

  • the app ships the wording of its automatic notifications as email templates, and which kinds of alert they cover;
  • each template comes in four languages, and each recipient gets their own language, falling back to English;
  • most alerts are emailed as well as delivered to the in-app inbox;
  • the reminder-style alerts need scheduled work switched on (with a link to Scheduled automation);
  • the source lives in an email-templates/ folder in src/sales/, src/service/ and src/revenue/.

The page does not list template names.

The tests that pin it

  • test/record-attachments-access.test.ts (new) boots the app's own options plus StorageServicePlugin. objectstack serve mounts that plugin, but the lean handle boot does not, so this is what makes the parent-derived half real in the test. A rep holding sales_rep and na_sales_team lists the attachment on the territory quote and not the one on an unreachable quote: GET /data/sys_attachment returns 200 with ['att_US']. The rep also cannot put a file on the unreachable quote: 403 PERMISSION_DENIED.
  • test/collaboration-capabilities.test.ts checks that every non-guest set that reads a files-enabled object holds sys_attachment read and no write bit, and that guest_portal names sys_attachment nowhere. Its header no longer says that no set needs the grant.
  • test/backfill-query-bodies.test.ts now selects every top-level scripts/*.ts whose source names the /api/v1/data/.../query route, so demo-staff.ts runs against FindDataRequestSchema beside the back-fill scripts. The stub answers the script's two writes (create-user and sys_user_position), and process.exit is stubbed.
  • The approval-hold branch of demo-staff.ts is measured on the live boxes above. No test pins it, because the probe's door returns no rows.

Ablations. These are one-shot checks run through objectstack's scripts/ablation-replace.mjs. Each restore is proven by the blob matching HEAD and an empty git diff HEAD. Both ran on HEAD ee14b076:

  • Deleting the sales_rep sys_attachment line turns lists the attachments of a quote they can read, and of no other red with 403 PERMISSION_DENIED, which is the measured defect. The structural test goes red naming sales_rep. Restored.
  • Putting filters, back into demo-staff.ts's query helper turns scripts/demo-staff.ts sends only query bodies the door accepts red with query.filters: too_small. Restored.

Gates

Each run was OS_VERIFY_LOCK_SLOT=hotcrm-2029 bash …/os-verify-lock.sh -c 'pnpm verify'.

  • 1c749299 (this branch, with origin/main f562142f merged): pnpm verify exit 0, VERDICT command-exit 0. Validate passed, typecheck, lint --strict, the i18n gate (0 issues), hygiene and the token ratchet were clean, build complete, and Test Files 173 passed (173) / Tests 3595 passed | 1 skipped (3596).
  • ecb93665 (the final head, with origin/main a2f2a40a merged): pnpm verify exit 1 at hygiene:tokens, failing with ✗ src/revenue business semantics is ~16,051 tokens; the ratchet ceiling is ~16,000 (over by ~51) and ✗ src/revenue authored total … over by ~161. Every step before it passed. On the same head, pnpm build && pnpm test finished with Test Files 1 failed | 172 passed (173), Tests 1 failed | 3608 passed | 1 skipped. The one failure is test/source-token-ratchet.test.ts, which is the same ratchet.
  • This red comes from main, not from this branch. git diff origin/main HEAD -- src/revenue is empty. Running scripts/check-source-token-ratchet.mjs on a git archive of origin/main a2f2a40a alone fails with the same two lines (exit 1). That commit is fix(quotes,hooks): refuse an acceptance an approval holds; draft a rep's contract; the capability gate and activity bubbles cross the caller's grants (runAs: 'system') #2035, which grew src/revenue/objects/quote.hook.ts. No ceiling is moved here. Once main is green again, this branch takes a fresh merge of main and pnpm verify is run again.
  • Not run locally: the Playwright e2e suite (pnpm test:e2e). pnpm verify does not include it.

Acceptance notes

These findings are reported to the seat in the os-dev report on #2029. None is fixed here.

  • Platform, upstream. The attachment parent gate admits any controlled_by_parent parent (measurements above). This blocks the upload and delete half of item 1.
  • Renderer, upstream. The console's Attachments panel shows Upload and a per-row delete to a person whose sys_attachment grant holds neither bit. Clicking Upload as na.rep uploads the bytes (presigned, PUT and complete all answer 200), then POST /api/v1/data/sys_attachment answers 403, and the panel shows "You don't have permission to do that."
  • App, same family as item 1. Every non-admin persona is refused the record Discussion panel: GET /api/v1/data/sys_comment answers 403 for na.rep, and the console shows "You don't have permission to view comments on this record." The enable.feeds note in src/sales/objects/index.ts still says any member can read and post on any thread.
  • App docs, same family as item 4, outside this claim's file surface. The Email templates section of content/docs/administration/automation*.mdx still says "HotCRM itself ships no email templates" and that each subject and body is written inline in its flow.
  • App docs, carried by the upload half. content/docs/guides/files-and-comments*.mdx says that record editors upload and delete. Until the platform fix lands, only administrators can.
  • Seed note. sales.seed.ts says the two large deals are born into approval. On one fresh boot they were, about 90 seconds after start. On another they were not after 3.5 minutes. No card.

Patch round 1 (appended by the repo:hotcrm seat)

Head 8be91fff: a clean merge of main b06081d8, which fixed the red main this PR inherited (#2037, PR #2039). No other change. pnpm verify green: 3,609 passed, 1 skipped; the token ratchet is clean. The upload / delete half of item 1 is #2038, blocked by objectstack-ai/objectstack#22455. Items 3, 5 and 6 stay on the card.


Generated by Claude Code

claude added 7 commits October 9, 2026 09:16
…pproval-locked deals

`pnpm demo:staff` stopped at step 3 with `POST /api/v1/data/crm_account/query
→ 400: Invalid query request`: its query helper sent `filters: []`, which
`FindDataRequestSchema` refuses (`query.filters` min_items), the shape #1999
removed from the back-fill scripts. An empty filter is now sent as no
`filters` key.

With that fixed, the same step stopped on a seeded box with a 409: the seed
deliberately puts two open deals (Acme Platform Upgrade, Wayne Enterprise
License) into `opportunity_approval` with `lockRecord: true`, and the
platform refuses the dev admin's ownership PATCH on a locked record. The
step now reads the pending approval requests and leaves those rows where
they are, reports them as held, and the census does not judge them.

`test/backfill-query-bodies.test.ts` now runs every top-level script that
posts to the data query door, demo-staff included, against the installed
query schema (#2029).

Claude-Session: https://claude.ai/code/session_012zh91QzFgePbkmuHnugLN3
Co-authored-by: Claude <noreply@anthropic.com>
…attachments

The quote's Attachments panel told `na.rep` "You don't have access to these
attachments." and `GET /api/v1/data/sys_attachment` answered 403
PERMISSION_DENIED for every demo persona: the platform's member baseline
names `sys_attachment` nowhere, and no permission set here granted it,
although the `enable.files` note said none had to. Each of the six sets
that can read a files-enabled object now grants `sys_attachment` read; the
platform's parent-derived filter keeps each person to the attachments of
the records they can read.

Upload and delete are deliberately not granted. On 17.7.0 the platform's
attach and delete gate asks `canEdit(parent)`, which answers true for every
controlled_by_parent parent: with the write bits granted, na.rep attached
a file to a quote they get 404 on and deleted the admin's file from a
contract they may not edit. That is reported upstream; the canonical note
and the structural test say so (#2029).

Claude-Session: https://claude.ai/code/session_012zh91QzFgePbkmuHnugLN3
Co-authored-by: Claude <noreply@anthropic.com>
The page said "this app authors no templates" in all three locales, while
`GET /api/v1/data/sys_email_template` serves 76 `crm.*` rows (19
notifications, four locales each) authored under
`src/{sales,service,revenue}/email-templates`. It now says what the
templates cover, that each recipient gets their own language, that most
are emailed as well as delivered to the inbox, that the reminder-style ones
need scheduled work switched on, and where the source lives (#2029).

Claude-Session: https://claude.ai/code/session_012zh91QzFgePbkmuHnugLN3
Co-authored-by: Claude <noreply@anthropic.com>
… the retired quote status word

`test/docs-object-term-consistency.test.ts` refuses 已到期 on every zh page:
it is the retired spelling of the quote status Expired (#802), and the
guard reads complete words, not who the sentence is about. The contract
alert now reads 合同到期 / 合約到期, the mechanism root the guard allows.

Claude-Session: https://claude.ai/code/session_012zh91QzFgePbkmuHnugLN3
Co-authored-by: Claude <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hotcrm Ignored Ignored Oct 9, 2026 10:43am UTC

Request Review

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 9, 2026 10:55
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 9, 2026
Merged via the queue into main with commit ac162c9 Oct 9, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd CI plumbing and the verification pipeline documentation Improvements or additions to documentation metadata Declarative metadata — schema, security posture, UI surfaces

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants