Postulo/postulo
Postulo/postulo
2
0
Fork 0
Code Issues 26 Pull requests Projects Releases 2 Packages 1 Wiki Activity Actions

Security tests and hardening lag the code: no isolation sweep, CSP untested, invites unhashed, proxy trust too wide, mail to any address #232

New issue
Closed
opened 2026-09-15 21:23:29 +00:00 by tiagoagueda · 1 comment
tiagoagueda commented 2026-09-15 21:23:29 +00:00
Owner
Copy link

Owner scoping is strong in practice: every view, form queryset, API router, search and export read in the audit narrows with for_user(). The tests and a few defaults have not kept up. Found in the 2026-09-15 code audit.

Tests

  1. The isolation sweep the threat model promises does not exist. THREAT-MODEL.md:14 says "the test suite sweeps every view and queryset for this". tests/test_ownership.py tests only the mixin against a test model, and isolation is otherwise tested app by app, so a new view with a pk can ship untested.
    → tests/security/test_isolation_sweep.py: walk the resolver (as test_page_coverage.py does), map each pattern with a pk to a factory that creates the object for another user, and assert GET and POST return 404. Keep an EXCUSED dict with reasons. Do the same for the API routes.

  2. tests/security/ misses recent boundaries. Nothing covers:

    • the Calendar page;
    • both ICS feeds;
    • the avatar view's "own picture or staff" rule (accounts/views.py:215-217);
    • the logo view;
    • export_download;
    • any outbound guard except PDF fetching.

    Rule 7 (THREAT-MODEL.md:75) asks for one test per endpoint.
    → test_feeds_and_pictures.py for cross-owner ICS, calendar, avatar and logo requests. Outbound tests are in the outbound-requests issue (#215).

  3. The CSP exists only in production settings. SECURE_CSP is defined only in config/settings/prod.py:77-87; test.py and dev.py have none, and tests/security/test_requests.py:146 only reads the settings dict. The e2e and axe suite runs without the policy, so an inline script or style= attribute passes CI and breaks in production.
    → Move SECURE_CSP into base.py, have the e2e suite fail on securitypolicyviolation events, and add worker-src 'self' and object-src 'none' explicitly now that /sw.js exists.

Hardening

  1. Invite tokens are stored in plain text (accounts/models.py:455-472, looked up directly at accounts/views.py:295), while recovery links store only a SHA-256 fingerprint and API tokens are hashed. This goes against rule 6 and the backup row of the threat model: anyone with a database backup can register with a pending invite, which also counts as proof of the email address.
    → Store a fingerprint and show the link once at creation, as recovery links do. Existing pending invites need re-issuing, and invite_list.html:48 must stop showing the link later.
  2. Every private network is trusted for X-Forwarded-For by default. core/proxy.py:37-45 trusts 10/8, 172.16/12, 192.168/16, fc00::/7 and fe80::/10, and rewrites REMOTE_ADDR. allauth's rate limits, the admin login limit and POSTULO_ENDPOINT_RATE all key on REMOTE_ADDR. Any LAN host, or a container on the same bridge, picks its own rate-limit identity. Under rootless Docker or Podman every internet client may arrive from a private gateway address (unconfirmed).
    → Default to loopback, have the operator name the proxy network, and show on Server settings → Overview which peer was trusted.
  3. The Email notifier mails any address, with no throttle. plugins/email/__init__.py:55-63 sends to config["to"], a plain email field with no check that the address is the person's, and the subject carries text the person controls. ConnectionTestView has no throttle; the text transport has per-account limits and mail has none.
    → Limit to to the account's verified addresses (allauth), or confirm a foreign address first, and throttle Test.
Owner scoping is strong in practice: every view, form queryset, API router, search and export read in the audit narrows with `for_user()`. The tests and a few defaults have not kept up. Found in the 2026-09-15 code audit. ## Tests 1. **The isolation sweep the threat model promises does not exist.** `THREAT-MODEL.md:14` says "the test suite sweeps every view and queryset for this". `tests/test_ownership.py` tests only the mixin against a test model, and isolation is otherwise tested app by app, so a new view with a pk can ship untested. → `tests/security/test_isolation_sweep.py`: walk the resolver (as `test_page_coverage.py` does), map each pattern with a pk to a factory that creates the object for another user, and assert GET and POST return 404. Keep an `EXCUSED` dict with reasons. Do the same for the API routes. 2. **`tests/security/` misses recent boundaries.** Nothing covers: - the Calendar page; - both ICS feeds; - the avatar view's "own picture or staff" rule (`accounts/views.py:215-217`); - the logo view; - `export_download`; - any outbound guard except PDF fetching. Rule 7 (`THREAT-MODEL.md:75`) asks for one test per endpoint. → `test_feeds_and_pictures.py` for cross-owner ICS, calendar, avatar and logo requests. Outbound tests are in the outbound-requests issue (#215). 3. **The CSP exists only in production settings.** `SECURE_CSP` is defined only in `config/settings/prod.py:77-87`; `test.py` and `dev.py` have none, and `tests/security/test_requests.py:146` only reads the settings dict. The e2e and axe suite runs without the policy, so an inline script or `style=` attribute passes CI and breaks in production. → Move `SECURE_CSP` into `base.py`, have the e2e suite fail on `securitypolicyviolation` events, and add `worker-src 'self'` and `object-src 'none'` explicitly now that `/sw.js` exists. ## Hardening 4. **Invite tokens are stored in plain text** (`accounts/models.py:455-472`, looked up directly at `accounts/views.py:295`), while recovery links store only a SHA-256 fingerprint and API tokens are hashed. This goes against rule 6 and the backup row of the threat model: anyone with a database backup can register with a pending invite, which also counts as proof of the email address. → Store a fingerprint and show the link once at creation, as recovery links do. Existing pending invites need re-issuing, and `invite_list.html:48` must stop showing the link later. 5. **Every private network is trusted for `X-Forwarded-For` by default.** `core/proxy.py:37-45` trusts 10/8, 172.16/12, 192.168/16, fc00::/7 and fe80::/10, and rewrites `REMOTE_ADDR`. allauth's rate limits, the admin login limit and `POSTULO_ENDPOINT_RATE` all key on `REMOTE_ADDR`. Any LAN host, or a container on the same bridge, picks its own rate-limit identity. Under rootless Docker or Podman every internet client may arrive from a private gateway address (unconfirmed). → Default to loopback, have the operator name the proxy network, and show on *Server settings → Overview* which peer was trusted. 6. **The Email notifier mails any address, with no throttle.** `plugins/email/__init__.py:55-63` sends to `config["to"]`, a plain email field with no check that the address is the person's, and the subject carries text the person controls. `ConnectionTestView` has no throttle; the text transport has per-account limits and mail has none. → Limit `to` to the account's verified addresses (allauth), or confirm a foreign address first, and throttle *Test*.
tiagoagueda added this to the 0.4.0 milestone 2026-09-15 21:33:27 +00:00
tiagoagueda commented 2026-09-19 09:39:02 +00:00
Author
Owner
Copy link

Landed on main in six commits, one per point:

  1. Isolation sweep — tests/security/test_isolation_sweep.py walks the resolver and, for every address that names a record (69 pages, 19 API routes), makes the other person's record and asks for it: GET and POST must be 404 or 405, an API route 404 with a body its schema accepts. A new pattern with an argument fails until it has a factory or a written excuse. Every existing one answers 404. The threat model now names the file instead of asserting the sweep exists.
  2. Feeds and pictures — tests/security/test_feeds_and_pictures.py: the calendar in its three shapes, both iCalendar feeds and the API's, the avatar rule, a company's logo, and the export archive (a POST, own records only).
  3. CSP — SECURE_CSP is in base.py, so every settings module sends it; worker-src, manifest-src and object-src are named. The browser suite runs under it and an autouse fixture fails any test on a policy refusal; axe goes in through the DevTools protocol and the text-spacing override through a constructed stylesheet, since both were <script>/<style> elements with content.
  4. Invites — the row keeps a SHA-256 fingerprint; Invite.issue hands the token back once, to the page that made it, which shows the link with the Copy button; the list has no link column; the session holds the fingerprint. The migration fingerprints existing tokens, so links already sent still open.
  5. Proxy trust — only loopback by default; POSTULO_TRUSTED_PROXIES names the network; Server settings → Overview shows the peer of the current request and whether it was trusted. The changelog says in bold what an upgrade behind a container or another host must set.
  6. Email notifier — to is a choice among the person's verified addresses (primary first); send and test both resolve the recipient the same way and refuse an address that stopped being theirs; a title is put on one line before it becomes a subject; Test on a connection and on the mail settings page is bounded by POSTULO_CONNECTION_TEST_RATE (10/h). Postulo hands user to a config_fields or test that asks for it by keyword, so the plugin contract is unchanged.
Landed on `main` in six commits, one per point: 1. **Isolation sweep** — `tests/security/test_isolation_sweep.py` walks the resolver and, for every address that names a record (69 pages, 19 API routes), makes the other person's record and asks for it: GET and POST must be 404 or 405, an API route 404 with a body its schema accepts. A new pattern with an argument fails until it has a factory or a written excuse. Every existing one answers 404. The threat model now names the file instead of asserting the sweep exists. 2. **Feeds and pictures** — `tests/security/test_feeds_and_pictures.py`: the calendar in its three shapes, both iCalendar feeds and the API's, the avatar rule, a company's logo, and the export archive (a POST, own records only). 3. **CSP** — `SECURE_CSP` is in `base.py`, so every settings module sends it; `worker-src`, `manifest-src` and `object-src` are named. The browser suite runs under it and an autouse fixture fails any test on a policy refusal; axe goes in through the DevTools protocol and the text-spacing override through a constructed stylesheet, since both were `<script>`/`<style>` elements with content. 4. **Invites** — the row keeps a SHA-256 fingerprint; `Invite.issue` hands the token back once, to the page that made it, which shows the link with the Copy button; the list has no link column; the session holds the fingerprint. The migration fingerprints existing tokens, so links already sent still open. 5. **Proxy trust** — only loopback by default; `POSTULO_TRUSTED_PROXIES` names the network; *Server settings → Overview* shows the peer of the current request and whether it was trusted. The changelog says in bold what an upgrade behind a container or another host must set. 6. **Email notifier** — `to` is a choice among the person's verified addresses (primary first); `send` and `test` both resolve the recipient the same way and refuse an address that stopped being theirs; a title is put on one line before it becomes a subject; *Test* on a connection and on the mail settings page is bounded by `POSTULO_CONNECTION_TEST_RATE` (10/h). Postulo hands `user` to a `config_fields` or `test` that asks for it by keyword, so the plugin contract is unchanged.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
0.3.0
v0.3.0
v0.2.1
v0.2.0
v0.1.0
Labels
Clear labels   
accessibility
Usable by everyone, including people with disabilities: one of the project's stated missions

  
authentication
Signing in, and proving who you are

  
breaking change
Changes behaviour or data in a way operators must act on

  
bug
Something does not do what it says it does

  
documentation
The wiki, the docs, or the words in the interface

  
enhancement
An improvement or a new capability

  
interface
The shape of the pages: what is where, and how it reads

  
internationalisation
Languages, regions, and the formats and symbols that go with them

  
observability
What an operator can see about a running instance: metrics, logs, health

  
security
Keeping the code and the data secure: one of the project's stated missions

  
tier
1
 Easy: small, self-contained, no prerequisites

  
tier
2
 High-severity security: account access, personal data, the boundaries

  
tier
3
 Foundations: prerequisite work that unblocks other issues

  
tier/4
Later: large features and long-horizon work

No labels
accessibility
authentication
breaking change
bug
documentation
enhancement
interface
internationalisation
observability
security
tier
1
tier
2
tier
3
tier/4
Milestone
Clear milestone
No items
No milestone
0.4.0
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Postulo/postulo#232
No description provided.