Shared-record leaks: archive, delete dialog, employer structure, merge preview (A1)

tiagoagueda commented

2026-10-17 13:26:22 +00:00

Owner

Fixes five findings of the 2026-10-16 security review that concern what a shared or group-owned record shows to people it is shared with. Part A1 of the SEC2 series.

What and why

#966 (High): the account archive exported every appointment at a shared or group-owned company. The appointments prefetch is now Appointment.objects.for_user(user), as the other children are, so build_document carries only the exporter's own meetings, and counts()["appointments"] equals what the file holds.
#973: the company merge preview listed every member's appointments. It filters by **mine, like the career rows beside it.
#958: delete confirmations counted other members' rows and were drawn for readers. CompanyDeleteView and PostingDeleteView count only the reader's own applications, appointments, history entries, captures, career rows, references and sent documents (postings and the contacts the reader can see stay record-level), and say without a number that what other people keep stays with them as a copy. The company and listing detail pages compute deletions.facts_for and draw the Delete and Merge buttons and the dialog only when sharing.manages.
#957: the employer-structure helpers were not narrowed. parent_of, children_of, group_of, in_a_group and companies_counted answer for the reader (visible companies only), the companies table's Part of column checks that the parent is visible, and CompanyForm refuses a parent whose audience could not see the company, on create as well as on edit (sharing.check_child_link).
#955: documentation. The threat model's sentence and the sharing sweep's docstring now name the contacts a person adds, and the appointments; the wiki's Accounts and invitations page says the same (commit in the wiki repo, not pushed).

Tests

tests/security/test_shared_leaks.py: the world fixture gained an owner's appointment with a secret note, added to SECRETS, so every surface test covers it; new tests for the archive (shared case and two members of a group, with the count), the merge plan, the delete dialog for a read-only reader and for a manager of a group's record, and both cases of the parent/child leak. They fail without the fix (7 of the 14 in that file). Full unit suite: 18,871 tests, green apart from catalogue-compiling tests that need messages.py compile in a fresh worktree (which passed once compiled). test_unscoped_queries caught that one baseline line was fixed, and it is removed.

Notes

Deviation from #958's text: the parent link refusal means an owner can no longer put a private company under their own shared one without sharing it with the same people first (it is what #957 asks for). A visible child list is narrowed anyway, so nothing leaks if one exists already.
The gdpr.references and erase_contact counts are left to A2 (#956, #961), which edits core/gdpr.py and the contact views.
Templates the e2e tests cover changed (company_detail.html, posting_detail.html, company_row.html): the owner still sees every button, so the CI browser job should be read for the company and listing pages.

Closes #966, Closes #957, Closes #958, Closes #973, Closes #955

🤖 Generated with Claude Code

Fixes five findings of the 2026-10-16 security review that concern what a shared or group-owned record shows to people it is shared with. Part A1 of the SEC2 series. ## What and why - **#966 (High): the account archive exported every appointment at a shared or group-owned company.** The `appointments` prefetch is now `Appointment.objects.for_user(user)`, as the other children are, so `build_document` carries only the exporter's own meetings, and `counts()["appointments"]` equals what the file holds. - **#973: the company merge preview listed every member's appointments.** It filters by `**mine`, like the career rows beside it. - **#958: delete confirmations counted other members' rows and were drawn for readers.** `CompanyDeleteView` and `PostingDeleteView` count only the reader's own applications, appointments, history entries, captures, career rows, references and sent documents (postings and the contacts the reader can see stay record-level), and say without a number that what other people keep stays with them as a copy. The company and listing detail pages compute `deletions.facts_for` and draw the Delete and Merge buttons and the dialog only when `sharing.manages`. - **#957: the employer-structure helpers were not narrowed.** `parent_of`, `children_of`, `group_of`, `in_a_group` and `companies_counted` answer for the reader (visible companies only), the companies table's *Part of* column checks that the parent is visible, and `CompanyForm` refuses a parent whose audience could not see the company, on create as well as on edit (`sharing.check_child_link`). - **#955: documentation.** The threat model's sentence and the sharing sweep's docstring now name the contacts a person adds, and the appointments; the wiki's *Accounts and invitations* page says the same (commit in the wiki repo, not pushed). ## Tests `tests/security/test_shared_leaks.py`: the `world` fixture gained an owner's appointment with a secret note, added to `SECRETS`, so every surface test covers it; new tests for the archive (shared case and two members of a group, with the count), the merge plan, the delete dialog for a read-only reader and for a manager of a group's record, and both cases of the parent/child leak. They fail without the fix (7 of the 14 in that file). Full unit suite: 18,871 tests, green apart from catalogue-compiling tests that need `messages.py compile` in a fresh worktree (which passed once compiled). `test_unscoped_queries` caught that one baseline line was fixed, and it is removed. ## Notes - Deviation from #958's text: the parent link refusal means an owner can no longer put a private company under their own shared one without sharing it with the same people first (it is what #957 asks for). A visible child list is narrowed anyway, so nothing leaks if one exists already. - The `gdpr.references` and `erase_contact` counts are left to A2 (#956, #961), which edits `core/gdpr.py` and the contact views. - Templates the e2e tests cover changed (`company_detail.html`, `posting_detail.html`, `company_row.html`): the owner still sees every button, so the CI browser job should be read for the company and listing pages. Closes #966, Closes #957, Closes #958, Closes #973, Closes #955 🤖 Generated with [Claude Code](https://claude.com/claude-code)

tiagoagueda added 3 commits

2026-10-17 13:26:22 +00:00

Keep other members' appointments out of the account archive and the merge preview da9da04a4f

The archive prefetched a company's whole appointments set, so a member of a group, or
anybody a service company is shared with, downloaded every other member's meetings with
their adviser, notes included. The prefetch is the exporter's own now, as the other
children are, and the count shown beside the download equals the file.

The company merge preview named every member's appointments at the company being merged
away; it names the viewer's own, like the career rows beside it.

Closes #966
Closes #973

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Narrow the delete confirmation and the employer structure to what the reader may see 2b6fd00c92

The consequences a deletion lists were counted over the whole record, so on a shared company
or listing they included other members' applications, meetings, history, captures and sent
documents. They are the reader's own now (postings and the contacts they can see stay
record-level), and a sentence without a number says that what other people keep stays with
them as a copy. The company and listing pages compute the facts, draw the Delete and Merge
buttons and write the dialog only when the reader manages the record, so a read-only grantee
is neither offered them nor sent the counts.

parent_of, children_of, group_of, in_a_group and companies_counted followed the links
without asking whether the reader could see what they led to, so a private company made the
child of a shared one was named, with its town, on the shared company's page for everybody
it is shared with, and a parent whose share was withdrawn stayed named. They answer for the
reader now, the companies table's Part of column does the same, and the company form refuses
a parent whose audience could not see the company (also when creating one), saying who.

Closes #958
Closes #957

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Say that the contacts a person adds stay theirs, and test the shared-record leaks

CI / Unit tests and coverage (Python 3.14) (pull_request) Failing after 31m51s

Details

CI / Checks: lint, migrations, catalogues, settings, built files (pull_request) Successful in 3m59s

Details

CI / Database tests on PostgreSQL (pull_request) Successful in 4m36s

Details

CI / Security audit (pull_request) Successful in 1m15s

Details

CI / Browser tests (Chromium) (pull_request) Failing after 47m40s

Details

12ba7e5305

The threat model's list of what stays a person's own on a shared record omits the contacts
they add at a company (and now the appointments); the sentence and the sharing sweep's
docstring say so. The shared-leaks world gains an owner's appointment with a secret note,
and tests cover the archive, the merge preview, the delete dialog and the parent/child
leak.

Closes #955

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

tiagoagueda added 1 commit

2026-10-17 14:58:06 +00:00

Run the checks again

CI / Unit tests and coverage (Python 3.14) (pull_request) Successful in 12m45s

Details

CI / Checks: lint, migrations, catalogues, settings, built files (pull_request) Successful in 2m25s

Details

CI / Database tests on PostgreSQL (pull_request) Successful in 3m19s

Details

CI / Security audit (pull_request) Successful in 53s

Details

CI / Browser tests (Chromium) (pull_request) Successful in 29m28s

Details

ca0a4984a5

The previous run of this branch stalled on an overloaded runner (memory and swap full,
load average 43): the unit job timed out in tests/security/test_outbound.py and the browser
job in tests/e2e/test_reflow.py, neither touched by this change. Both pass locally.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

tiagoagueda merged commit fbb4ee11d9 into main

2026-10-17 16:59:58 +00:00

tiagoagueda referenced this pull request from a commit

2026-10-17 16:59:59 +00:00

Merge pull request 'Shared-record leaks: archive, delete dialog, employer structure, merge preview (A1)' (#993) from sec2-a1-shared-leaks into main

tiagoagueda referenced this pull request

2026-10-19 10:56:10 +00:00

SEC2 index: security review of 2026-10-16 (86 issues across the core, plugins and extensions) #992

Rows
Columns

Shared-record leaks: archive, delete dialog, employer structure, merge preview (A1) #993

What and why

Tests

Notes