Security review: the remaining core issues (#959-#985)

tiagoagueda commented

2026-10-18 22:21:20 +00:00

Owner

The thirteen remaining open core issues of the 2026-10-16 security review, one commit per issue.

#959 geojson.read and the JSON-LD reader catch RecursionError (a document nested thousands deep); logos._walk is iterative. Tests: a deeply nested GeoJSON upload gets the refusal, a deeply nested JSON-LD block ends in the "nothing" sentence.
#960 logos.from_url spends the allowance of by, the person acting (no longer the company's owner or the shared None bucket); the logo dialog's apply_logo(..., spend=False) ends its double charge. Tests count whose allowance is spent for a grantee and for a group-owned company.
#962 notifications.slow.link applies absolute_url's precedence: POSTULO_PUBLIC_URL, else the stored base, else the bare path.
#963 no ATTENDEE and no notes in any calendar file (interview and appointment; the DAV plugin writes the same lines); the people met stay named as text. Both help texts say the notes are left out. The goldens of the old writer lose the same two lines. The DAV plugin's own test and METHOD:PUBLISH choice belong in postulo-dav.
#964 a box on the upload form, ticked by default, cuts EXIF/XMP/IPTC/comments out of a JPEG and EXIF/text/time chunks out of a PNG by the file's structure, with no re-encode (same pixels; the stored checksum is of what is kept).
#965 docs/THREAT-MODEL.md says the referee rules bind the shipped themes and the sections value, and that a plugin theme is handed full records and trusted (#820).
#976 the request id is judged by proxy.request_origin(), the peer that delivered the request, so a trusted proxy's X-Request-ID survives its X-Forwarded-For.
#977 POSTULO_PROXY_ADDR is left whenever a trusted peer sent any forwarding header; /healthz and the probe middleware read it through proxy.was_forwarded().
#979 logs.scrub() applies logredact.redact() first, so the JSON log scrubs password-reset and email-confirmation keys from one list (logredact joins the foundation layer).
#980 seed_demo --password is a flag meaning "prompt"; the value comes from POSTULO_SEED_PASSWORD; CI's PostgreSQL job passes it through env:.
#982 the kept-back notices are composed with htmlstrings.sentence and the feature's own label (each of the three plugins exposes LABEL); no |safe on a translation; a lint allows |safe in offline.html and partials/messages.html only.
#983 messages_tool.foreign_markup (html.parser) is part of message_problems: a draft with markup the English lacks is left out of the .mo, a reviewed one fails check; weblate.py configure adds the safe-html flag (not run); the "you are here" tooltip is built from an element with textContent.
#985 the board's fold links write hx-vals with as_json; a template lint refuses an hx-vals/hx-headers built by interpolation.

Closes #959, #960, #962, #963, #964, #965, #976, #977, #979, #980, #982, #983, #985

Deviations from the issue text

#964 keeps an APP1 segment holding only the JPEG's orientation, so a photograph taken sideways is not shown sideways; the issue's "getexif() is empty" holds for any photo without an orientation tag. Also dropped: APP3, COM, and data after the image's end (JPEG); tIME (PNG).
#963 drops notes from every file outright rather than gating them on a parameter (no UI added).
#976 the issue names proxy.request_origin() as existing; it did not, so it was added (and describe() uses it).
#979 redact runs before the route pattern (not after), so the two routes the JSON log already named keep their <token> mask.
#983 #1003/#1004 already compared well-formed tags; this adds what a browser reads differently (comments, unterminated tags, attributes and addresses, a stray <).
Six core msgids change (#982), one (#963); their translations are redone in Weblate. No msgstr written.

Tested

Whole unit suite with -n 2: 19397 passed (one commit later, the two failures it showed, test_import_layers and test_changelog, are fixed and re-run). E2E, locally, one file at a time: test_map.py (new tooltip test, red on the old JS), test_board_fold.py, test_company_social_profiles.py, and in test_accessibility.py the company form at 2560 px and a new upload-form test, both schemes. The CI browser job should be read carefully: templates and JS touched are the board, the company form, the contact and profile pages, the upload form and the map tooltip.

🤖 Generated with Claude Code

The thirteen remaining open core issues of the 2026-10-16 security review, one commit per issue. - **#959** `geojson.read` and the JSON-LD reader catch `RecursionError` (a document nested thousands deep); `logos._walk` is iterative. Tests: a deeply nested GeoJSON upload gets the refusal, a deeply nested JSON-LD block ends in the "nothing" sentence. - **#960** `logos.from_url` spends the allowance of `by`, the person acting (no longer the company's owner or the shared `None` bucket); the logo dialog's `apply_logo(..., spend=False)` ends its double charge. Tests count whose allowance is spent for a grantee and for a group-owned company. - **#962** `notifications.slow.link` applies `absolute_url`'s precedence: `POSTULO_PUBLIC_URL`, else the stored base, else the bare path. - **#963** no `ATTENDEE` and no notes in any calendar file (interview and appointment; the DAV plugin writes the same lines); the people met stay named as text. Both help texts say the notes are left out. The goldens of the old writer lose the same two lines. The DAV plugin's own test and `METHOD:PUBLISH` choice belong in postulo-dav. - **#964** a box on the upload form, ticked by default, cuts EXIF/XMP/IPTC/comments out of a JPEG and EXIF/text/time chunks out of a PNG by the file's structure, with no re-encode (same pixels; the stored checksum is of what is kept). - **#965** `docs/THREAT-MODEL.md` says the referee rules bind the shipped themes and the `sections` value, and that a plugin theme is handed full records and trusted (#820). - **#976** the request id is judged by `proxy.request_origin()`, the peer that delivered the request, so a trusted proxy's `X-Request-ID` survives its `X-Forwarded-For`. - **#977** `POSTULO_PROXY_ADDR` is left whenever a trusted peer sent any forwarding header; `/healthz` and the probe middleware read it through `proxy.was_forwarded()`. - **#979** `logs.scrub()` applies `logredact.redact()` first, so the JSON log scrubs password-reset and email-confirmation keys from one list (`logredact` joins the foundation layer). - **#980** `seed_demo --password` is a flag meaning "prompt"; the value comes from `POSTULO_SEED_PASSWORD`; CI's PostgreSQL job passes it through `env:`. - **#982** the kept-back notices are composed with `htmlstrings.sentence` and the feature's own label (each of the three plugins exposes `LABEL`); no `|safe` on a translation; a lint allows `|safe` in `offline.html` and `partials/messages.html` only. - **#983** `messages_tool.foreign_markup` (html.parser) is part of `message_problems`: a draft with markup the English lacks is left out of the `.mo`, a reviewed one fails `check`; `weblate.py configure` adds the `safe-html` flag (not run); the "you are here" tooltip is built from an element with `textContent`. - **#985** the board's fold links write `hx-vals` with `as_json`; a template lint refuses an `hx-vals`/`hx-headers` built by interpolation. Closes #959, #960, #962, #963, #964, #965, #976, #977, #979, #980, #982, #983, #985 ## Deviations from the issue text - **#964** keeps an APP1 segment holding only the JPEG's orientation, so a photograph taken sideways is not shown sideways; the issue's "`getexif()` is empty" holds for any photo without an orientation tag. Also dropped: APP3, COM, and data after the image's end (JPEG); `tIME` (PNG). - **#963** drops notes from every file outright rather than gating them on a parameter (no UI added). - **#976** the issue names `proxy.request_origin()` as existing; it did not, so it was added (and `describe()` uses it). - **#979** `redact` runs before the route pattern (not after), so the two routes the JSON log already named keep their `<token>` mask. - **#983** #1003/#1004 already compared well-formed tags; this adds what a browser reads differently (comments, unterminated tags, attributes and addresses, a stray `<`). - Six core msgids change (#982), one (#963); their translations are redone in Weblate. No msgstr written. ## Tested Whole unit suite with `-n 2`: 19397 passed (one commit later, the two failures it showed, `test_import_layers` and `test_changelog`, are fixed and re-run). E2E, locally, one file at a time: `test_map.py` (new tooltip test, red on the old JS), `test_board_fold.py`, `test_company_social_profiles.py`, and in `test_accessibility.py` the company form at 2560 px and a new upload-form test, both schemes. **The CI browser job should be read carefully:** templates and JS touched are the board, the company form, the contact and profile pages, the upload form and the map tooltip. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

tiagoagueda added 13 commits

2026-10-18 22:21:20 +00:00

Refuse JSON nested thousands deep instead of answering 500 (#959 ) f798c3971d

CPython's json raises RecursionError on a document nested a few thousand
levels, which is not a ValueError, so neither geojson.read nor the logo
finder caught it. geojson.read now refuses it with the usual sentence; the
JSON-LD reader skips the block, and _walk is iterative so a document nested
as deep as the parser allows cannot exhaust the stack either.

Closes #959

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

Charge a logo address to the person who pressed the button (#960 ) 935e57efb2

logos.from_url spent the allowance of company.owner. For a company shared
with a read/write grantee that was somebody else's allowance, and for a
group-owned company (owner None) one bucket shared by every group company.
from_url now takes `by`, the person acting, and refuses to spend without
one; the company form passes request.user, and the logo dialog, which has
already spent for the press, tells apply_logo not to spend again, which
also ends its double charge.

Closes #960

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

Build capture notification links on POSTULO_PUBLIC_URL first (#962 ) f559275e8f

#747 made absolute_url prefer the operator's POSTULO_PUBLIC_URL over the
request's Host, but the errand that delivers capture_received and
capture_batch kept the older order: the Host stored by the capture API won.
slow.link now applies the same precedence: the public address, else the
stored base, else the bare path.

Closes #962

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

Keep private notes and attendees out of calendar files (#963 ) abdd9186fa

An interview's file wrote the person's preparation notes into DESCRIPTION
and each contact with an address as an ATTENDEE, against the form's own
"nothing here is sent to anybody". The same lines go to the subscribed feed,
the downloads and, through event_lines, a sync plugin's calendar, which may
be shared; many calendar programs offer to send an attendee updates.

Both components (interview and appointment) now write no ATTENDEE and no
notes; the people met are still named as text in the description. The
help text on both notes fields says the notes are left out of the calendar
files. The goldens of the old writer lose the same two lines.

The DAV plugin's own test of the attendee line and its METHOD:PUBLISH
choice belong in postulo-dav.

Closes #963

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

Say what a theme is really handed in the threat model (#965 ) 37cb0c153e

The referee paragraph promised that a plugin theme is never handed the
entry or the contact, while the template context carries the whole CV and
letter and the plugin row of the same document says plugin themes get full
records (#820). The paragraph now says that the shipped themes and the
`sections` value obey the two rules and a plugin theme is trusted.

Closes #965

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

Judge the request id by the proxy that delivered the request (#976 ) f15dca0f67

RequestIDMiddleware asked whether the rewritten REMOTE_ADDR was a trusted
proxy; behind a trusted proxy that also sets X-Forwarded-For it is the
visitor's address, so the proxy's X-Request-ID was thrown away and the
proxy's, gunicorn's and Postulo's logs could not be laid side by side.
proxy.request_origin() names the peer that delivered the request (the
logic describe() already had) and the middleware uses it.

Closes #976

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

Treat any forwarding header as proof of a proxy in /healthz (#977 ) 52c2abb341

TrustedProxyMiddleware left POSTULO_PROXY_ADDR only when X-Forwarded-For
arrived. A loopback reverse proxy that sets just X-Forwarded-Proto (the
documented nginx configuration) therefore looked like the container's own
probe: answered before the host check, and handed the exact version. The
marker is now left whenever a trusted peer sent any forwarding header,
the REMOTE_ADDR rewrite stays conditional on X-Forwarded-For, and
healthz and the probe middleware read the marker through was_forwarded().

Closes #977

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

Scrub the same one-time secrets from the JSON log as the access log (#979 ) cd40675c6e

logs.scrub() knew two routes (invitation, recovery) while logredact, which
the access log uses, knew five. A 500 while serving the password-reset key
page or the email-confirmation page therefore wrote the live key into
postulo.log, which administrators read and /logs hands to a collector.
scrub() now applies logredact.redact() first and then its own route
pattern, so there is one list and a future addition lands in one place.

Closes #979

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

Read seed_demo's password from the environment or a prompt (#980 ) 24e6852b67

--password took the password as an argument: in the shell's history, in ps
for the length of the run and in any container log that echoes the command.
It is now a flag meaning "prompt"; the value comes from POSTULO_SEED_PASSWORD
or, absent that, is made up and not shown. A string handed to call_command
as a keyword (scripts/screenshots.py) still works, since Python has no shell
history. CI's PostgreSQL job passes the demo password through env:.

Closes #980

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

Stop printing a translated sentence as markup (#982 ) fce187b564

The company form printed Block.kept_back_contact with |safe, so whatever a
translator wrote in that msgstr reached the page unescaped, while the
profile and contact pages printed the same sentence escaped and showed a
literal <em>. The sentences now carry a {setting} slot instead of markup;
Block.kept_back_html and kept_back_contact_html fill it through
htmlstrings.sentence (translation escaped, only the named slot filled) with
the feature's own label, which each of the three plugins now exposes as
LABEL so it is translated once. The three templates print the property
without |safe, and tests/test_template_lint.py refuses |safe anywhere but
offline.html and partials/messages.html.

Six core msgids change (their translations are redone in Weblate).

Closes #982

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

Encode the board's hx-vals with as_json (#985 ) 6aa7be902a

The fold links built a JSON document by dropping an HTML-escaped string into
a quoted JSON string, so their safety rested on a filter in a view two files
away. columns() now carries the value as a dict and the template writes it
with as_json, as the map attributes do. A lint beside HTMX_SCRIPTING refuses
an hx-vals or hx-headers value with interpolation in it unless the only
thing in it is `{{ x|as_json }}`.

Closes #985

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

Refuse markup a translation adds, and draw the map tooltip as text (#983 ) 3cadadbbf0

Django prints a blocktranslate as template source and Leaflet writes a
string tooltip with innerHTML, so a msgstr that carries markup the English
does not is injected content. #1003/#1004 already compared the well-formed
tags of source and translation; that missed what a browser reads
differently from a regular expression (a comment, an unterminated tag, an
attribute or address in a tag the English has).

messages_tool.foreign_markup parses source and form with html.parser: every
(tag, attribute) pair and every href/src-like value of the form must be the
source's, and the form may not hold more '<' than the source. It is part of
message_problems, so a draft with it is left out of the .mo like one that
lost a placeholder, a reviewed one fails `check` and CI, and
tests/test_translations.py covers it. `weblate.py configure` adds Weblate's
safe-html flag to each component's check flags (not run here; it is a
server write). The "you are here" tooltip gets an element with textContent,
not a string, and an e2e test puts <b> in the attribute.

Closes #983

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

Cut a photograph's metadata out of an uploaded document by default (#964 )

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

Details

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

Details

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

Details

CI / Security audit (pull_request) Successful in 48s

Details

CI / Browser tests (Chromium) (pull_request) Successful in 28m49s

Details

6180e08b35

An uploaded PNG or JPEG was decoded to prove it was a picture and then
stored byte for byte, so a scanned diploma that is really a phone photograph
kept its GPS position, capture time and camera, and was attached to mail for
an employer and copied to every store, while every avatar and logo is
re-encoded to lose exactly that.

The upload form gets a box, ticked by default, that removes the metadata
losslessly: JPEG segments APP1 (Exif, XMP), APP3, APP13 (IPTC) and COM, and
anything after the image's end; PNG chunks eXIf, tEXt, iTXt, zTXt and tIME.
Nothing is re-encoded, so the pixels are identical and the stored checksum
is of what is kept. One deliberate exception to "empty": a JPEG keeps an
APP1 holding only its orientation, or a photograph taken sideways would be
shown sideways. PDF, Word and OpenDocument files still carry what they carry
(#878 is that surface); the help text says so. The edit form has no box: a
stored file cannot be changed (#217).

Closes #964

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

tiagoagueda merged commit 42be520482 into main

2026-10-19 06:11:12 +00:00

tiagoagueda referenced this pull request from a commit

2026-10-19 06:11:14 +00:00

Merge pull request 'Security review: the remaining core issues (#959-#985)' (#1010) from sec2-remaining-core 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

Security review: the remaining core issues (#959-#985) #1010

Deviations from the issue text

Tested