PST-045: Owner scoping rests on each write path: several read helpers and generic relations take ids without an owner filter #774

New issue

Closed

opened 2026-10-10 10:56:17 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-10 10:56:17 +00:00

Owner

Found in the security audit of 2026-10-10 (SECURITY-AUDIT-2026-10-10.md), run against main at c30851b from 8 to 10 October 2026, as PST-045. Severity: Low.

The text below is the audit's entry, unchanged apart from links to the other issues it names. The finding was produced by automated review and checked by a second automated pass; re-check it before acting on it.

Severity: Low
Category: authorization / missing invariant on a generic relation
Location: src/postulo/documents/models.py (463-500; 72-87); src/postulo/documents/proofs.py (84-103); src/postulo/resume/translating.py (96); src/postulo/applications/views.py (530-532,779,926,1012,1166,1193,1217,1549,1558,1569); src/postulo/jobs/models.py (Contact.company); src/postulo/applications/suggestions.py (24-67); src/postulo/plugins/models.py (332-354)
Found by: ownership review of applications, ownership review of documents, ownership review of jobs, ownership review of plugins-views, ownership review of resume
Related: PST-044 (#773)

Who can do it, and what they need. Not reachable by an ordinary account through any current request path: CVAddItemsView validates posted ids against MultipleChoiceField choices built from cv.owner's rows (forms.py 439-466), verified by a scratch test for experience, education and project ids as well as the sweep's reference case; the importer maps refs through rows it created for the same user. …

How it works.

A writer stores CVItem(owner=A, cv=A's CV, content_type=Experience, object_id=<B's experience pk>) -- nothing in CVItem.save() or clean() refuses it, unlike UploadedDocument.check_proves, which raises for a foreign proves target on every save.
A opens the CV page, preview, draft, export or any format download.
with_entries() prefetches the target with model.objects.all() (no owner filter) and build_sections renders cv_item.item as an entry.
B's employment entry (organisation, role, dates, highlights) is printed into A's CV and into a PDF A can send anywhere.

Impact. A single unguarded write anywhere turns into another person's career entry being rendered and exported. The threat model's own answer for the parallel proves link was a save-time check ('no way of writing the link -- the form, the importer, the shell -- can point at somebody else's entry'); the older and more widely used CVItem link has no such check.

Evidence.

models.py:463-475
class CVItem(OwnedModel):
    ...
    content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
    object_id = models.PositiveIntegerField()
    item = GenericForeignKey("content_type", "object_id")

models.py:449-458 (with_entries)
    querysets = [
        model.objects.prefetch_related("skills") if ... else model.objects.all()
        for model in apps.get_app_config("resume").get_models()
        if issubclass(model, ResumeItem)
    ]

models.py:742-751 (the guard the upload link has and this one lacks)
    def check_proves(self) -> None:
        ...
        if not proofs.is_own(self):
            raise ValidationError(_("That entry is not one of yours.")
…

Verification (verdict: confirmed; severity after verification: Low). The quoted sentence 'no way of writing the link -- the form, the importer, the shell -- can point at somebody else's entry' comes from the check_proves docstring in models.py. It is not in docs/THREAT-MODEL.md, which does not mention it. File and lines are right (models.py:449-475). Request-path exploitability is nil today. The finding is that the invariant lives only in one form. Remediation: validate in CVItem.save() or clean() that the target model has owner_id == self.owner_id, as check_proves does, and restrict with_entries querysets to the CV owner.

Suggested remediation. Mirror check_proves on CVItem: in save() verify the target model is a ResumeItem subclass and model.objects.filter(pk=object_id, owner_id=owner_id).exists(), and that cv.owner_id == owner_id; optionally scope the with_entries prefetch querysets with owner_id=cv.owner_id as a second line. Add a test like tests/security/test_proofs.py::test_a_save_refuses_a_link_to_another_persons_entry for CVItem.

Also covered by this issue.

renders_of() (CV.renders / CoverLetter.renders) lists snapshots by source link only, not by owner (src/postulo/documents/models.py (72-87); Informational). Limited: sent_to text and a date of another account's snapshot would be drawn on A's page; the file itself stays unreachable. By contrast rendering.exported_on_its_own() (rendering.py:969-971) and RenderedDocument.previous() (models.py:1136) do add owner, so the same question is answered …
Helper queries over generic links (proofs.counts_for, archiving.copies_for) resolve by content id without an owner filter (src/postulo/documents/proofs.py (84-103); Informational). Negligible information (a count, a store's label/status) and no file access; recorded so the synthesis can say these helpers are not independently owner-scoped.
Translation and proof lookups are keyed by (content_type, object_id) with no owner filter: a defence-in-depth gap against the 'every query goes through for_user()' rule (src/postulo/resume/translating.py (96); Informational). None today. Would become stored cross-account content injection into a CV (Translation) or a misleading count (proofs) if a scoped-write invariant were ever broken.
Child rows (Reminder, Interview, Offer) are trusted through their application FK without re-scoping, so an owner mismatch would cross accounts in both directions (src/postulo/applications/views.py (530-532,779,926,1012,1166,1193,1217,1549,1558,1569); Informational). Given the precondition: cross-account read of one application's outstanding reminders (text and dates) and of its role/company through the fragment and edit pages; injection of another account's text into the owner's page; cross-account deletion through the cascade. …
Cross-owner foreign keys are prevented only by each write path; the jobs pages follow reverse relations unscoped (src/postulo/jobs/models.py (Contact.company); Informational). None presently; one unscoped writer anywhere in the process would turn several jobs pages into cross-account reads and deletes without any further bug.
A plugin-filed Suggestion is never checked for owner/application agreement at filing time (src/postulo/applications/suggestions.py (24-67); Informational). No path for an ordinary account; a plugin defect would show one person's role and employer in another's suggestion queue, and accepting it would raise a server error instead of a quiet 404. Recorded so the synthesis can say the boundary was looked at.
SyncLink.bind/for_record/of_model do not check that the record belongs to the connection's owner (src/postulo/plugins/models.py (332-354); Informational). No reachable path for an account holder; a defensive assert at the one chokepoint plugins must use would turn a plugin's scoping bug into a logged failure instead of a cross-account push.

Found in the security audit of 2026-10-10 (`SECURITY-AUDIT-2026-10-10.md`), run against `main` at `c30851b` from 8 to 10 October 2026, as **PST-045**. Severity: **Low**. The text below is the audit's entry, unchanged apart from links to the other issues it names. The finding was produced by automated review and checked by a second automated pass; re-check it before acting on it. --- - **Severity:** Low - **Category:** authorization / missing invariant on a generic relation - **Location:** `src/postulo/documents/models.py` (463-500; 72-87); `src/postulo/documents/proofs.py` (84-103); `src/postulo/resume/translating.py` (96); `src/postulo/applications/views.py` (530-532,779,926,1012,1166,1193,1217,1549,1558,1569); `src/postulo/jobs/models.py` (Contact.company); `src/postulo/applications/suggestions.py` (24-67); `src/postulo/plugins/models.py` (332-354) - **Found by:** ownership review of applications, ownership review of documents, ownership review of jobs, ownership review of plugins-views, ownership review of resume - **Related:** PST-044 (#773) **Who can do it, and what they need.** Not reachable by an ordinary account through any current request path: `CVAddItemsView` validates posted ids against `MultipleChoiceField` choices built from `cv.owner`'s rows (forms.py 439-466), verified by a scratch test for experience, education and project ids as well as the sweep's reference case; the importer maps refs through rows it created for the same user. … **How it works.** 1. A writer stores CVItem(owner=A, cv=A's CV, content_type=Experience, object_id=<B's experience pk>) -- nothing in `CVItem.save()` or `clean()` refuses it, unlike `UploadedDocument.check_proves`, which raises for a foreign `proves` target on every save. 2. A opens the CV page, preview, draft, export or any format download. 3. `with_entries()` prefetches the target with `model.objects.all()` (no owner filter) and `build_sections` renders `cv_item.item` as an entry. 4. B's employment entry (organisation, role, dates, highlights) is printed into A's CV and into a PDF A can send anywhere. **Impact.** A single unguarded write anywhere turns into another person's career entry being rendered and exported. The threat model's own answer for the parallel `proves` link was a save-time check ('no way of writing the link -- the form, the importer, the shell -- can point at somebody else's entry'); the older and more widely used CVItem link has no such check. **Evidence.** ```text models.py:463-475 class CVItem(OwnedModel): ... content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) object_id = models.PositiveIntegerField() item = GenericForeignKey("content_type", "object_id") models.py:449-458 (with_entries) querysets = [ model.objects.prefetch_related("skills") if ... else model.objects.all() for model in apps.get_app_config("resume").get_models() if issubclass(model, ResumeItem) ] models.py:742-751 (the guard the upload link has and this one lacks) def check_proves(self) -> None: ... if not proofs.is_own(self): raise ValidationError(_("That entry is not one of yours.") … ``` **Verification** (verdict: confirmed; severity after verification: Low). The quoted sentence 'no way of writing the link -- the form, the importer, the shell -- can point at somebody else's entry' comes from the check_proves docstring in models.py. It is not in docs/THREAT-MODEL.md, which does not mention it. File and lines are right (models.py:449-475). Request-path exploitability is nil today. The finding is that the invariant lives only in one form. Remediation: validate in CVItem.save() or clean() that the target model has owner_id == self.owner_id, as check_proves does, and restrict with_entries querysets to the CV owner. **Suggested remediation.** Mirror `check_proves` on CVItem: in `save()` verify the target model is a ResumeItem subclass and `model.objects.filter(pk=object_id, owner_id=owner_id).exists()`, and that `cv.owner_id == owner_id`; optionally scope the `with_entries` prefetch querysets with `owner_id=cv.owner_id` as a second line. Add a test like tests/security/test_proofs.py::test_a_save_refuses_a_link_to_another_persons_entry for CVItem. **Also covered by this issue.** - **`renders_of()` (CV.renders / CoverLetter.renders) lists snapshots by source link only, not by owner** (`src/postulo/documents/models.py` (72-87); Informational). Limited: `sent_to` text and a date of another account's snapshot would be drawn on A's page; the file itself stays unreachable. By contrast `rendering.exported_on_its_own()` (rendering.py:969-971) and `RenderedDocument.previous()` (models.py:1136) do add `owner`, so the same question is answered … - **Helper queries over generic links (`proofs.counts_for`, `archiving.copies_for`) resolve by content id without an owner filter** (`src/postulo/documents/proofs.py` (84-103); Informational). Negligible information (a count, a store's label/status) and no file access; recorded so the synthesis can say these helpers are not independently owner-scoped. - **Translation and proof lookups are keyed by (content_type, object_id) with no owner filter: a defence-in-depth gap against the 'every query goes through for_user()' rule** (`src/postulo/resume/translating.py` (96); Informational). None today. Would become stored cross-account content injection into a CV (Translation) or a misleading count (proofs) if a scoped-write invariant were ever broken. - **Child rows (Reminder, Interview, Offer) are trusted through their `application` FK without re-scoping, so an owner mismatch would cross accounts in both directions** (`src/postulo/applications/views.py` (530-532,779,926,1012,1166,1193,1217,1549,1558,1569); Informational). Given the precondition: cross-account read of one application's outstanding reminders (text and dates) and of its role/company through the fragment and edit pages; injection of another account's text into the owner's page; cross-account deletion through the cascade. … - **Cross-owner foreign keys are prevented only by each write path; the jobs pages follow reverse relations unscoped** (`src/postulo/jobs/models.py` (Contact.company); Informational). None presently; one unscoped writer anywhere in the process would turn several jobs pages into cross-account reads and deletes without any further bug. - **A plugin-filed Suggestion is never checked for owner/application agreement at filing time** (`src/postulo/applications/suggestions.py` (24-67); Informational). No path for an ordinary account; a plugin defect would show one person's role and employer in another's suggestion queue, and accepting it would raise a server error instead of a quiet 404. Recorded so the synthesis can say the boundary was looked at. - **SyncLink.bind/for_record/of_model do not check that the record belongs to the connection's owner** (`src/postulo/plugins/models.py` (332-354); Informational). No reachable path for an account holder; a defensive assert at the one chokepoint plugins must use would turn a plugin's scoping bug into a logged failure instead of a cross-account push.

tiagoagueda added the

bug

security

labels

2026-10-10 10:56:18 +00:00

tiagoagueda referenced this issue

2026-10-10 10:57:48 +00:00

PST-044: The owner-folder guard is missing on rendered-document and export downloads, and is a raw prefix test elsewhere #773

tiagoagueda added this to the 0.6.0 milestone

2026-10-10 10:57:48 +00:00

tiagoagueda referenced this issue

2026-10-10 10:58:40 +00:00