# 2026-04-04 Security Sweep — OWASP Top 10 Audit and Fixes

> **Session date:** 2026-04-04
> **Agents used:** Shield (security audit), Forge (PHP fixes), Pixel (theme preprocess fixes)
> **ClickUp tickets:** none
> **Branch:** make-package

---

## Summary

Shield conducted a full OWASP Top 10 audit across all custom modules and themes. Seven security issues were identified and fixed across two custom modules and the `naomi_2025` theme, covering sensitive data exposure via debug logging (A09), insecure direct object exposure via plain-text password fields (A05), and cross-site scripting risks in theme preprocess functions (A03). A missing `hash_salt` in the Lando settings file was also flagged and addressed.

---

## Work Completed

### Shield — OWASP Top 10 Security Audit

**What was done:**
Full audit of all custom PHP modules and theme preprocess files against the OWASP Top 10. Issues categorised by severity and OWASP category. All findings passed to Forge and Pixel for remediation.

**Files audited:**
- `web/modules/custom/mailjet_webform/src/Plugin/WebformHandler/MailjetListHandler.php`
- `web/modules/custom/mailjet_api/src/MailjetApiHandler.php`
- `web/modules/custom/mailjet_api/src/Form/MailjetApiAdminSettingsForm.php`
- `web/themes/custom/naomi_2025/theme-hooks/preprocess-paragraph.php`
- `web/themes/custom/naomi_2025/theme-hooks/preprocess-node.php`
- `web/themes/custom/naomi_2025/theme-hooks/preprocess-field.php`
- `web/sites/default/settings.lando.php`

**Decisions made:**
- Issues rated high/medium only — no low-severity cosmetic findings were actioned in this session.

---

### Forge — Custom Module Security Fixes

**What was done:**
Fixed two high-severity and one medium-severity issue in the Mailjet custom modules.

**Files changed:**
- `web/modules/custom/mailjet_webform/src/Plugin/WebformHandler/MailjetListHandler.php` — replaced `print_r($data)` PII debug log with a field-keys-only log; removed API key value from error log output (HIGH — OWASP A09: Security Logging and Monitoring Failures)
- `web/modules/custom/mailjet_api/src/MailjetApiHandler.php` — removed `print_r` of full API response in debug mode to prevent sensitive data appearing in logs (MEDIUM — OWASP A09)
- `web/modules/custom/mailjet_api/src/Form/MailjetApiAdminSettingsForm.php` — changed `api_key_secret` form element from `#type => 'textfield'` to `#type => 'password'` so the key is masked in the UI; added empty-submit handling in `validateForm()` and `submitForm()` so that submitting the form without entering a new secret preserves the existing stored key rather than overwriting it with an empty string (MEDIUM — OWASP A05: Security Misconfiguration)

**Decisions made:**
- For the admin settings form, when the password field is left blank on submit, `validateForm()` now reads the stored secret from config and injects it into the form state so `submitForm()` can save it unchanged. This avoids a separate "clear key" UX path while keeping the fix simple.
- `print_r` debug output was removed entirely rather than gated behind a config flag, to eliminate the risk of it being re-enabled in production.

---

### Pixel — Theme Preprocess Security Fixes

**What was done:**
Added output escaping to preprocess functions in the `naomi_2025` theme that were passing unescaped values directly to `Markup::create()`, creating reflected XSS risk.

**Files changed:**
- `web/themes/custom/naomi_2025/theme-hooks/preprocess-paragraph.php` — added `Xss::filterAdmin()` around embed code before passing to `Markup::create()`; added `use Drupal\Component\Utility\Xss;` statement (MEDIUM — OWASP A03: Injection)
- `web/themes/custom/naomi_2025/theme-hooks/preprocess-node.php` — added `Html::escape()` around `$node->label()` (L32) and `$field_display_title` (L99) before `Markup::create()` (MEDIUM — OWASP A03)
- `web/themes/custom/naomi_2025/theme-hooks/preprocess-field.php` — added `Html::escape()` around `$text` in the `advanced_header_field` loop (MEDIUM — OWASP A03)

**Decisions made:**
- `Xss::filterAdmin()` was used for the embed code field (rather than `Html::escape()`) because embed code may contain legitimate HTML tags that should survive sanitisation. `filterAdmin()` allows a safe subset of HTML while stripping script and event-handler attributes.
- `Html::escape()` was used for plain text values (`node label`, `display title`, header field text) where no markup is expected.

---

### Atlas — Settings Security Fix

**What was done:**
Added a `hash_salt` placeholder to the Lando-local settings file.

**Files changed:**
- `web/sites/default/settings.lando.php` — added `$settings['hash_salt']` placeholder with inline instructions for generating a value; prevents Drupal from falling back to an insecure or empty hash salt in local development (MEDIUM — OWASP A05)

**Decisions made:**
- Placeholder value and generation instructions added inline rather than generating a committed salt, to avoid a shared/known salt value landing in version control.

---

## Testing

- [x] `lando drush cr` — cache rebuilt successfully after all changes
- [ ] PHPCS — not run this session; recommended before merge
- [ ] Manual test: Mailjet admin settings form — verify existing key is preserved on blank-password submit
- [ ] Manual test: Embed paragraph — verify legitimate embed HTML renders correctly after `Xss::filterAdmin()` is applied
- [ ] Manual test: Node titles with special characters — verify correct escaping without double-encoding

## Notes for Future Sessions

- PHPCS should be run before `make-package` is merged — some files may have minor style violations introduced alongside the security fixes.
- The `hash_salt` in `settings.lando.php` is a placeholder only. Each developer should generate and set their own value locally — instructions are in the file.
- No low-severity findings were addressed in this session. A follow-up sweep is recommended once the branch is merged.
