Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"drupal/config_update": "^2@alpha",
"drupal/core-composer-scaffold": "~11.3.7",
"drupal/core-recommended": "~11.3.7",
"drupal/csp": "^2.1",
"drupal/devel": "^5.5.0",
"drupal/diff": "^1.10",
"drupal/entity_clone": "^2.1@beta",
Expand Down
50 changes: 49 additions & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions config/default/core.extension.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ module:
content_moderation: 0
contextual: 0
crop: 0
csp: 0
ctools: 0
datetime: 0
datetime_range: 0
Expand Down
64 changes: 64 additions & 0 deletions config/default/csp.settings.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
report-only:
enable: false
directives: { }
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
reporting:
plugin: none
enforce:
enable: true
directives:
default-src:
base: self
script-src:
base: self
sources:
- https://www.googletagmanager.com
- https://www.gstatic.com
- https://www.recaptcha.net
- https://www.google.com
- https://cdnjs.cloudflare.com
- https://cdn.jsdelivr.net/gh/cferdinandi/tabby@12.0.3/dist/js/tabby.min.js
- https://unpkg.com/@popperjs/core@2.11.6/dist/umd/popper.js
- https://unpkg.com/tippy.js@6.3.7/dist/tippy.umd.js
object-src:
base: none
style-src:
base: self
flags:
- unsafe-inline
Comment on lines +24 to +27
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider migrating away from unsafe-inline for styles.

The unsafe-inline flag in style-src allows arbitrary inline styles, which degrades CSP protection. While this is a common trade-off in Drupal implementations where inline styles are prevalent, it opens the door to CSS-based attacks and potential data exfiltration.

Consider migrating to nonce-based or hash-based styles in the long term to strengthen the security posture.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@config/default/csp.settings.yml` around lines 24 - 27, The CSP currently
permits inline styles via the style-src flags entry containing "unsafe-inline";
update the policy by removing "unsafe-inline" from the style-src flags and
migrate to a nonce- or hash-based approach: replace the flags entry with either
a runtime-generated nonce token (e.g., add a style-src "nonce-{nonce}"
placeholder in the config) or with specific style hashes, implement server-side
nonce generation (inject the same nonce into the CSP header and into any inline
<style> or style attributes in templates), and ensure the configuration keys
referenced (style-src, base, flags) are updated to emit the nonce/hash value so
the CSP header and rendered pages remain in sync.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not applying in this PR. 'style-src' with 'unsafe-inline' is inherited from the previous 'seckit' policy and kept unchanged to preserve 1:1 behaviour during the migration. Drupal core and CivicTheme emit inline styles that would need hashes or nonces assigned across many render paths. Tracking as a follow-up to migrate 'style-src' to nonces once an audit of inline styles is complete.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexSkrypnyk, understood — preserving 1:1 parity with the existing seckit policy during migration is a reasonable rationale for keeping unsafe-inline in style-src for now, and a full audit across Drupal core + CivicTheme render paths is the right prerequisite before switching to nonces or hashes.

Would you like me to open a follow-up GitHub issue to track the style-src unsafe-inline → nonce/hash migration? I can include a link back to this PR and comment for context.

sources:
- https://cdnjs.cloudflare.com/ajax/libs/highlight.js/
- https://fonts.googleapis.com/
- https://cdn.jsdelivr.net/gh/cferdinandi/tabby@12.0.3/dist/css/tabby-ui.min.css
- https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.12/codemirror.css
- https://unpkg.com/tippy.js@6.3.7/dist/tippy.css
- https://cdnjs.cloudflare.com/ajax/libs/select2/4.0.13/css/select2.min.css
img-src:
base: self
sources:
- 'data:'
media-src:
base: self
frame-src:
base: self
sources:
- https://www.youtube.com
- https://www.recaptcha.net
- https://www.google.com
frame-ancestors:
base: none
font-src:
base: self
sources:
- https://fonts.gstatic.com
connect-src:
base: self
sources:
- https://www.googletagmanager.com
- https://www.google-analytics.com
- https://www.recaptcha.net
- https://www.google.com
upgrade-insecure-requests: true
Comment on lines +8 to +60
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add base-uri and form-action directives to strengthen CSP.

The CSP configuration is missing two important security directives:

  1. base-uri: Controls allowed base URLs for relative URL resolution. Without this directive, an attacker could inject a <base> tag to redirect relative URLs to malicious endpoints. Recommend adding base-uri: { base: self } to restrict base URLs to the same origin.

  2. form-action: Restricts where forms can be submitted. Without this directive, forms could potentially be submitted to attacker-controlled endpoints. Recommend adding form-action: { base: self } (or a more restrictive policy based on your requirements).

These directives provide defense-in-depth protection against base tag injection and form hijacking attacks.

🛡️ Proposed additions
     connect-src:
       base: self
       sources:
         - https://www.googletagmanager.com
         - https://www.google-analytics.com
         - https://www.recaptcha.net
         - https://www.google.com
+    base-uri:
+      base: self
+    form-action:
+      base: self
     upgrade-insecure-requests: true
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
directives:
default-src:
base: self
script-src:
base: self
sources:
- https://www.googletagmanager.com
- https://www.gstatic.com
- https://www.recaptcha.net
- https://www.google.com
- https://cdnjs.cloudflare.com
- https://cdn.jsdelivr.net/gh/cferdinandi/tabby@12.0.3/dist/js/tabby.min.js
- https://unpkg.com/@popperjs/core@2.11.6/dist/umd/popper.js
- https://unpkg.com/tippy.js@6.3.7/dist/tippy.umd.js
object-src:
base: none
style-src:
base: self
flags:
- unsafe-inline
sources:
- https://cdnjs.cloudflare.com/ajax/libs/highlight.js/
- https://fonts.googleapis.com/
- https://cdn.jsdelivr.net/gh/cferdinandi/tabby@12.0.3/dist/css/tabby-ui.min.css
- https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.12/codemirror.css
- https://unpkg.com/tippy.js@6.3.7/dist/tippy.css
- https://cdnjs.cloudflare.com/ajax/libs/select2/4.0.13/css/select2.min.css
img-src:
base: self
sources:
- 'data:'
media-src:
base: self
frame-src:
base: self
sources:
- https://www.youtube.com
- https://www.recaptcha.net
- https://www.google.com
frame-ancestors:
base: none
font-src:
base: self
sources:
- https://fonts.gstatic.com
connect-src:
base: self
sources:
- https://www.googletagmanager.com
- https://www.google-analytics.com
- https://www.recaptcha.net
- https://www.google.com
upgrade-insecure-requests: true
directives:
default-src:
base: self
script-src:
base: self
sources:
- https://www.googletagmanager.com
- https://www.gstatic.com
- https://www.recaptcha.net
- https://www.google.com
- https://cdnjs.cloudflare.com
- https://cdn.jsdelivr.net/gh/cferdinandi/tabby@12.0.3/dist/js/tabby.min.js
- https://unpkg.com/@popperjs/core@2.11.6/dist/umd/popper.js
- https://unpkg.com/tippy.js@6.3.7/dist/tippy.umd.js
object-src:
base: none
style-src:
base: self
flags:
- unsafe-inline
sources:
- https://cdnjs.cloudflare.com/ajax/libs/highlight.js/
- https://fonts.googleapis.com/
- https://cdn.jsdelivr.net/gh/cferdinandi/tabby@12.0.3/dist/css/tabby-ui.min.css
- https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.12/codemirror.css
- https://unpkg.com/tippy.js@6.3.7/dist/tippy.css
- https://cdnjs.cloudflare.com/ajax/libs/select2/4.0.13/css/select2.min.css
img-src:
base: self
sources:
- 'data:'
media-src:
base: self
frame-src:
base: self
sources:
- https://www.youtube.com
- https://www.recaptcha.net
- https://www.google.com
frame-ancestors:
base: none
font-src:
base: self
sources:
- https://fonts.gstatic.com
connect-src:
base: self
sources:
- https://www.googletagmanager.com
- https://www.google-analytics.com
- https://www.recaptcha.net
- https://www.google.com
base-uri:
base: self
form-action:
base: self
upgrade-insecure-requests: true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@config/default/csp.settings.yml` around lines 8 - 60, Add the missing CSP
directives under the top-level "directives" mapping: introduce a new "base-uri"
directive with "base: self" and a new "form-action" directive with "base: self"
(or a more restrictive source list if needed). Update the same structure used by
existing directives (like "default-src", "script-src", "style-src") so
"base-uri: { base: self }" and "form-action: { base: self }" appear alongside
them to lock down allowed base URLs and permitted form submission targets.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not applying in this PR. The scope of this change is an intentional 1:1 migration of the existing 'seckit' policy to 'drupal/csp' - no source list is widened or narrowed, and no new directive is introduced. 'base-uri' and 'form-action' were not present in the previous policy and adding them here risks breaking known-good flows (e.g., third-party form submissions) without testing. Tracking as a follow-up hardening step.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexSkrypnyk, understood — keeping this PR as a strict 1:1 migration is the right call. Introducing untested directive changes mid-migration is a good way to accidentally break third-party form flows or other integrations. The follow-up hardening step makes sense once the migrated policy is confirmed stable.

Would you like me to open a GitHub issue to track the addition of base-uri and form-action directives as a follow-up CSP hardening task?

reporting:
plugin: uri
options:
uri: /report-csp-violation
2 changes: 1 addition & 1 deletion config/default/seckit.settings.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
seckit_xss:
csp:
checkbox: true
checkbox: false
vendor-prefix:
x: true
webkit: true
Expand Down
37 changes: 37 additions & 0 deletions tests/behat/features/csp.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
@csp @p2
Feature: Content Security Policy

As a site owner
I want the Content-Security-Policy header to include a per-request nonce
and match the previously enforced policy
So that BigPipe and other Drupal inline scripts are not blocked by CSP
and no source is silently widened or narrowed during the migration

@api
Scenario: CSP header contains a nonce for anonymous users
Given I am an anonymous user
When I go to the homepage
Then the response status code should be 200
And the response header "Content-Security-Policy" should contain the value "'nonce-"

@api
Scenario: CSP header contains a nonce for authenticated users
Given I am logged in as a user with the "administrator" role
When I go to the homepage
Then the response status code should be 200
And the response header "Content-Security-Policy" should contain the value "'nonce-"

Comment on lines +11 to +23
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add a regression test for nonce rotation across requests.

Current checks confirm nonce presence but not that the nonce changes per request.

Suggested scenario addition
+  `@api`
+  Scenario: CSP nonce changes between requests
+    Given I am an anonymous user
+    When I go to the homepage
+    Then capture the response header "Content-Security-Policy" value as "csp_header_1"
+    When I go to the homepage
+    Then capture the response header "Content-Security-Policy" value as "csp_header_2"
+    And the extracted nonce from "csp_header_1" should not equal the extracted nonce from "csp_header_2"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/behat/features/csp.feature` around lines 11 - 23, Add a regression
Behat scenario that verifies CSP nonce rotation by making two consecutive
requests to the homepage and asserting the "Content-Security-Policy" header
contains a nonce on each request and that the nonce values differ; reuse the
existing steps ("Given I am an anonymous user" / "Given I am logged in as a user
with the \"administrator\" role", "When I go to the homepage", "Then the
response header \"Content-Security-Policy\" should contain the value
\"'nonce-\"") but extend the scenario to capture the full nonce substring from
the header on the first request, perform a second request, capture the second
nonce, and assert the two nonce strings are not equal to ensure rotation for
both anonymous and authenticated cases.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not applying in this PR. The steps in the suggestion are not provided by the DrupalExtension Mink step library used here, so adopting the scenario would require a custom FeatureContext step to extract the nonce from a header and compare two captured values across requests. Nonce rotation is already guaranteed by the 'drupal/csp' 'Nonce' service, which regenerates the random value on each request, and the module ships its own 'NonceSettingTest' functional coverage. Keeping this out of scope here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexSkrypnyk, understood — that's a fair call. The drupal/csp module's built-in NonceSettingTest functional coverage already guarantees per-request nonce rotation, and adding the scenario here would require custom FeatureContext step definitions outside the scope of this PR. Thanks for the clear explanation!


✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/website PR: 166
File: tests/behat/features/csp.feature:11-23
Timestamp: 2026-04-22T09:43:11.276Z
Learning: In the `drevops/website` repository, Behat tests use the DrupalExtension Mink step library. Custom header-capture and cross-request value comparison steps are not available out of the box and would require a custom FeatureContext implementation. Avoid suggesting Behat scenarios that rely on such non-standard steps unless a custom FeatureContext already exists in the project.

Learnt from: AlexSkrypnyk
Repo: drevops/website PR: 166
File: tests/behat/features/csp.feature:11-23
Timestamp: 2026-04-22T09:43:11.276Z
Learning: In `drevops/website`, CSP per-request nonce rotation is guaranteed by the `drupal/csp` module's `Nonce` service and is covered by the module's own `NonceSettingTest` functional test. There is no need to add a Behat regression scenario for nonce rotation in `tests/behat/features/csp.feature`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@api
Scenario: CSP policy preserves previously allowed sources
Given I am an anonymous user
When I go to the homepage
Then the response status code should be 200
And the response header "Content-Security-Policy" should contain the value "default-src 'self'"
And the response header "Content-Security-Policy" should contain the value "object-src 'none'"
And the response header "Content-Security-Policy" should contain the value "frame-ancestors 'none'"
And the response header "Content-Security-Policy" should contain the value "report-uri /report-csp-violation"
And the response header "Content-Security-Policy" should contain the value "https://www.googletagmanager.com"
And the response header "Content-Security-Policy" should contain the value "https://www.recaptcha.net"
And the response header "Content-Security-Policy" should contain the value "https://www.youtube.com"
And the response header "Content-Security-Policy" should contain the value "https://fonts.gstatic.com"
And the response header "Content-Security-Policy" should contain the value "https://www.google-analytics.com"
10 changes: 1 addition & 9 deletions tests/behat/features/seckit.feature
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,10 @@ Feature: Seckit
In order to improve security and protect against common vulnerabilities

@api
Scenario: Check for HSTS and CSP headers
Scenario: Seckit emits the expected non-CSP security headers
Given I am an anonymous user
When I go to the homepage
Then the response status code should be 200
And the response header "Content-Security-Policy" should contain the value "connect-src 'self' https://www.googletagmanager.com https://www.google-analytics.com https://www.recaptcha.net https://www.google.com;"
And the response header "Content-Security-Policy" should contain the value "default-src 'self';"
And the response header "Content-Security-Policy" should contain the value "font-src 'self' https://fonts.gstatic.com;"
And the response header "Content-Security-Policy" should contain the value "img-src 'self' data"
And the response header "Content-Security-Policy" should contain the value "media-src 'self'"
And the response header "Content-Security-Policy" should contain the value "report-uri /report-csp-violation"
And the response header "Content-Security-Policy" should contain the value "script-src 'self' https://www.googletagmanager.com https://www.gstatic.com https://www.recaptcha.net https://www.google.com https://cdnjs.cloudflare.com https://cdn.jsdelivr.net/gh/cferdinandi/tabby@12.0.3/dist/js/tabby.min.js https://unpkg.com/@popperjs/core@2.11.6/dist/umd/popper.js https://unpkg.com/tippy.js@6.3.7/dist/tippy.umd.js;"
And the response header "Content-Security-Policy" should contain the value "style-src 'self' https://cdnjs.cloudflare.com/ajax/libs/highlight.js/ 'unsafe-inline' https://fonts.googleapis.com/ https://cdn.jsdelivr.net/gh/cferdinandi/tabby@12.0.3/dist/css/tabby-ui.min.css https://cdnjs.cloudflare.com/ajax/libs/codemirror/5.65.12/codemirror.css https://unpkg.com/tippy.js@6.3.7/dist/tippy.css https://cdnjs.cloudflare.com/ajax/libs/select2/4.0.13/css/select2.min.css;"
And the response header "Strict-Transport-Security" should contain the value "max-age=31536000"
And the response header "Strict-Transport-Security" should contain the value "includeSubDomains"
And the response header "Strict-Transport-Security" should contain the value "preload"
Expand Down
3 changes: 3 additions & 0 deletions tests/phpunit/Drupal/EnvironmentSettingsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ public function testEnvironmentLocal(): void {
$config['shield.settings']['shield_enable'] = FALSE;
$config['system.logging']['error_level'] = 'all';
$config['system.performance']['cache']['page']['max_age'] = 900;
$config['csp.settings']['enforce']['directives']['upgrade-insecure-requests'] = FALSE;
$config['purge_control.settings']['disable_purge'] = TRUE;
$config['purge_control.settings']['purge_auto_control'] = FALSE;
$config['seckit.settings']['seckit_xss']['csp']['upgrade-req'] = FALSE;
Expand Down Expand Up @@ -431,6 +432,7 @@ public function testEnvironmentLocalContainer(): void {
$config['shield.settings']['shield_enable'] = FALSE;
$config['system.logging']['error_level'] = 'all';
$config['system.performance']['cache']['page']['max_age'] = 900;
$config['csp.settings']['enforce']['directives']['upgrade-insecure-requests'] = FALSE;
$config['purge_control.settings']['disable_purge'] = TRUE;
$config['purge_control.settings']['purge_auto_control'] = FALSE;
$config['seckit.settings']['seckit_xss']['csp']['upgrade-req'] = FALSE;
Expand Down Expand Up @@ -484,6 +486,7 @@ public function testEnvironmentGha(): void {
$config['shield.settings']['shield_enable'] = FALSE;
$config['system.logging']['error_level'] = 'all';
$config['system.performance']['cache']['page']['max_age'] = 900;
$config['csp.settings']['enforce']['directives']['upgrade-insecure-requests'] = FALSE;
$config['purge_control.settings']['disable_purge'] = TRUE;
$config['purge_control.settings']['purge_auto_control'] = FALSE;
$config['seckit.settings']['seckit_xss']['csp']['upgrade-req'] = FALSE;
Expand Down
17 changes: 17 additions & 0 deletions web/modules/custom/do_base/do_base.module
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

declare(strict_types=1);

use Drupal\csp\Csp;
use Drupal\Core\Site\Settings;

/**
Expand All @@ -20,3 +21,19 @@ function do_base_mail_alter(array &$message): void {
$message['send'] = FALSE;
}
}

/**
* Implements hook_page_attachments().
*/
function do_base_page_attachments(array &$attachments): void {
// Attach a CSP nonce to script-src on every page so that Drupal core's
// inline scripts (BigPipe placeholders, drupalSettings, etc.) continue to
// run under a strict Content-Security-Policy. The fallback 'unsafe-inline'
// is only used by browsers that do not support CSP3 nonces; modern
// browsers ignore it when a nonce is present.
if (!class_exists(Csp::class)) {
return;
}
$attachments['#attached']['csp_nonce']['script'] = [Csp::POLICY_UNSAFE_INLINE];
$attachments['#attached']['library'][] = 'csp/nonce';
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
13 changes: 13 additions & 0 deletions web/sites/default/includes/modules/settings.csp.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

/**
* @file
* CSP settings.
*/

declare(strict_types=1);

if ($settings['environment'] === ENVIRONMENT_CI || $settings['environment'] === ENVIRONMENT_LOCAL) {
// Disable CSP locally and in CI as we do not serve the site over HTTPS.
$config['csp.settings']['enforce']['directives']['upgrade-insecure-requests'] = FALSE;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
Loading