Fixes from Automated Security Review #596

Merged
tobru merged 12 commits from security/review-fixes into main 2026-07-06 09:01:31 +00:00
Owner
No description provided.
ServiceInstanceDeleteView set permission_type = "delete" but had no
AutoPermissionRequiredMixin in its MRO, so that attribute was dead code
and the only enforced check was core.view_organization (org membership).
The ServiceInstance model policy requires is_organization_admin for
delete, so any MEMBER could POST the delete URL and tear down the live
Kubernetes resource.

Add AutoPermissionRequiredMixin (mirroring ServiceInstancePauseView /
ServiceInstanceUpdateView) so the model's delete rule is enforced. Add a
regression test asserting MEMBER gets 403 while ADMIN/OWNER reach the view.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The dynamic array widget rendered its hidden input value with
`{{ value_list_json|safe }}`. json.dumps does not HTML-escape, so a
user-supplied array item like `"><img src=x onerror=...>` broke out of
the value="..." attribute and injected live markup. The value is
persisted to the K8s spec and re-rendered as the form's initial value,
yielding stored, cross-user XSS (e.g. a member's payload firing in an
admin's session).

Drop `|safe` so Django autoescapes the attribute. The widget JS reads
`input.value`, which the browser decodes back to the original JSON, so
form submission is unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SERVALA_ENVIRONMENT defaulted to "development", so a missing or
misspelled env var set DEBUG=True. DEBUG also gates SESSION_COOKIE_SECURE
and the cached template loader, meaning one absent var in production
silently exposed full tracebacks (settings/secret values) and disabled
the secure session cookie.

Default to "production" so the consequential switch fails safe. Local
dev is unaffected: .env sets SERVALA_ENVIRONMENT=development explicitly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
InvitationDeleteView.get_queryset filtered only by accepted_by, and
OrganizationInvitation has no ScopedManager, so django-scopes did not
constrain it. The object was resolved purely by URL pk while the
permission check tested change_organization against the URL-slug org.
An admin of org A could therefore delete (and, via the unguarded GET
confirmation page, disclose the invited email of) any pending invitation
of org B by placing B's pk in A's URL.

Scope get_queryset to request.organization so a foreign pk 404s, gate
both GET and POST via dispatch, and guard the missing-object path. Add
tests for cross-tenant blocking, same-org deletion, and member denial.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SESSION_COOKIE_SECURE was hardened but CSRF_COOKIE_SECURE was left at its
default of False, so the CSRF cookie could be transmitted over plain HTTP
and captured on the wire. Tie it to `not DEBUG` to match the session
cookie.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The only transport hardening in staging/production was
SECURE_PROXY_SSL_HEADER. Without HSTS an initial HTTP request to this
SSO-fronted portal is downgradeable (sslstrip/MITM). Add
SECURE_SSL_REDIRECT and a 1-year HSTS policy (include-subdomains +
preload) in the staging/production block, and enable
SECURE_CONTENT_TYPE_NOSNIFF globally.

If TLS termination and the HTTP->HTTPS redirect happen at the ingress,
these are still safe (the redirect uses the forwarded-proto header) and
ensure the app emits HSTS regardless.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The OSB Basic Auth check compared the shared username/password with `==`,
a theoretical timing side-channel on the machine-to-machine secret. Use
hmac.compare_digest (str-wrapped so the missing-credential False sentinel
compares safely) to match the constant-time comparison used elsewhere.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Acceptance was keyed only on the invitation secret, so any authenticated
user with the link (e.g. a forwarded invite) could claim the membership
at the invitation's role, including ADMIN/OWNER. Enforce that the logged
in user's email matches the invited address before creating the
membership, hide the accept button on mismatch, and add tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The plausible event was emitted as `{{ ...|safe }}` from json.dumps in a
<script> block, and form help_text as `{{ field.help_text|safe }}`.
Neither escapes </script> or HTML, so admin/cluster-sourced content
(catalog names, CRD OpenAPI descriptions) rendered as raw markup.

Emit the plausible payload with the json_script filter (HTML-escaped,
parsed via JSON.parse) and drop |safe from help_text. Intentional-HTML
help still renders because mark_safe/format_html produce SafeStrings that
autoescape leaves untouched; only raw strings are now escaped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Expert-form fields under HIDDEN_FIELDS (e.g. spec.writeConnectionSecretToRef)
were rendered as HiddenInput but stayed editable, so a crafted POST could
inject arbitrary values that create_instance used verbatim. Mark them
disabled so Django ignores submitted data and uses the initial value.
Hardcoded params are already re-applied server-side, so disabling the
hidden inputs does not change their behaviour.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
validate_nested_data was a no-op (TODO/pass), so the expert form did no
schema validation of the reconstructed spec. Run the OpenAPI/JSON schema
validator and log any mismatches, restoring a defense-in-depth layer with
observability.

It logs rather than blocks: K8s schemas use nullable /
x-kubernetes-int-or-string / preserve-unknown-fields that plain JSON
Schema would false-reject, and the K8s API server is the authoritative
validator. Blocking (add_error) can be enabled once validated against
live CRD schemas.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(security): escape instance fields in support ticket HTML
All checks were successful
Tests / test (push) Successful in 1m22s
a4abbeb2b9
build_instance_details interpolated instance/namespace names (including
user-controlled display_name) straight into an HTML string sent to Odoo.
Escape each value so a display_name like <script>...</script> cannot
inject markup if the ticket content is rendered in a web context.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tobru changed title from WIP: Security Review to Fixes from Automated Security Review 2026-07-06 09:01:26 +00:00
tobru merged commit 8163c5336a into main 2026-07-06 09:01:31 +00:00
tobru deleted branch security/review-fixes 2026-07-06 09:01:31 +00:00
Sign in to join this conversation.
No description provided.