The base-uri policy is missing

Attaching patch https://git.drupalcode.org/project/seckit/-/merge_requests/5.diff for easing composer workflows.

Sorry for the noise, I'm sure there must be a better way to get versioned changes from Bitbucket merge requests.

Also, updating issue status and version.

there must be a better way to get versioned changes from Bitbucket merge requests.

I'm not sure about Bitbucket (d.o. merge requests are using Gitlab, AFAIK), but I think https://github.com/cweagans/composer-patches/pull/388 is the most recent work on allowing merge request patches to not be a security risk.

See also https://github.com/cweagans/composer-patches/issues/347

In the meantime, traditional patch uploads are still the way to go.

7.x-1.x patch for the testbot.

Thanks @jweowu! I meant GitLab, indeed.

Patch in #7 works and looks good to me.

You can't RTBC your own patch :)

(as a matter of clarification, I take no credit of patch #7)

Whoops, I see now. Back to RTBC for #6 / #7.

In fact this is Needs Work -- I kicked off a test earlier against 2.x, and I see that it failed.

You could refer to #10 (which is for 7.x-1.x) regarding how you might be able to update the existing tests (I'm not sure offhand how similar the the D8+ tests are, though).

Trying to fix tests.

Trying again

       $form['seckit_xss[csp][default-src]'] = 'self';
+      $form['seckit_xss[csp][base-uri]'] = 'self';

Huh. That's a bug, isn't it? Or bad test data, at any rate.

AFAIK any 'self' value includes the single quotes, so in code that will be:

"'self'" rather than 'self'

Perhaps a fix for this should be rolled separately.

I'm not sure if it's a bug to be honest, but it's confusing for sure :)

Any base-uri is being outputted with single quotes, and I think that's fine.

However, this is not consistent with default-src. In the tests, "self" without quotes is being printed, and I'm not sure if that's valid in CSP. But that's unrelated to this patch.

Yes, the error is with the default-src code, so I think we're copying a mistake in the patch here.

I suspect there are other lines like that in the existing tests, so it would be good to patch that first.

(But regardless, let's use the correct "'self'" in the code for base-url.)

Any base-uri is being outputted with single quotes, and I think that's fine.

Ah, no, this is wrong:

+    if ($csp_base_uri) {
+      $directives[] = "base-uri '$csp_base_uri'";
+    }

You can't single-quote that value. It's a source list, so there may be multiple values, and any quoting needs to be explicit in the individual elements of that list.

https://w3c.github.io/webappsec-csp/#source-lists

interdiff.txt is against #7.

Missed one.

Fixing form field description.

Unrelated, but as said, we probably want a follow-up issue for the wrong default-src test data.

Also, I think its current field description is not correct:

> Specify security policy for all types of content, which are not specified further (frame-ancestors excepted). Default is 'self'.

In this module configuration, the default value is empty. If it's not set or empty, this directive is omitted and will not be sent to the browser, and then the browser will not enforce any default restriction for other directives (that is different to setting it to "'self'"). So 'self' is neither the default configuration in this module, nor the default behaviour in CSP: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Securi...

I would change the default-src description, removing the last sentence ("Default is 'self'.").

Adding the related issue for the default-src has wrong description.

Patch 3098417-26.patch is partly rejected in current stable release 2.0.1. Looks like rejected code is in /tests/src/Functional/SecKitTestCaseTest.php

Do we have a working patch for 2.0.1?

Rejected code

--- tests/src/Functional/SecKitTestCaseTest.php
+++ tests/src/Functional/SecKitTestCaseTest.php
@@ -97,11 +97,12 @@ class SecKitTestCaseTest extends BrowserTestBase {
       'seckit_xss[csp][child-src]' => '*',
       'seckit_xss[csp][font-src]' => '*',
       'seckit_xss[csp][connect-src]' => '*',
+      'seckit_xss[csp][base-uri]' => '*',
       'seckit_xss[csp][report-uri]' => $this->reportPath,
       'seckit_xss[csp][upgrade-req]' => TRUE,
     ];
     $this->drupalPostForm('admin/config/system/seckit', $form, t('Save configuration'));
-    $expected = 'default-src *; script-src *; object-src *; style-src *; img-src *; media-src *; frame-src *; frame-ancestors *; child-src *; font-src *; connect-src *; report-uri ' . base_path() . $this->reportPath . '; upgrade-insecure-requests';
+    $expected = "default-src *; script-src *; object-src *; style-src *; img-src *; media-src *; frame-src *; frame-ancestors *; child-src *; font-src *; connect-src *; base-uri *; report-uri " . base_path() . $this->reportPath . '; upgrade-insecure-requests';
     $this->assertSession()->responseHeaderEquals('Content-Security-Policy', $expected);
     $this->assertSession()->responseHeaderEquals('X-Content-Security-Policy', $expected);
     $this->assertSession()->responseHeaderEquals('X-WebKit-CSP', $expected);
@@ -126,11 +127,12 @@ class SecKitTestCaseTest extends BrowserTestBase {
       'seckit_xss[csp][child-src]' => '*',
       'seckit_xss[csp][font-src]' => '*',
       'seckit_xss[csp][connect-src]' => '*',
+      'seckit_xss[csp][base-uri]' => '*',
       'seckit_xss[csp][report-uri]' => $this->reportPath,
       'seckit_xss[csp][upgrade-req]' => TRUE,
     ];
     $this->drupalPostForm('admin/config/system/seckit', $form, t('Save configuration'));
-    $expected = 'default-src *; script-src *; object-src *; style-src *; img-src *; media-src *; frame-src *; frame-ancestors *; child-src *; font-src *; connect-src *; report-uri ' . base_path() . $this->reportPath . '; upgrade-insecure-requests';
+    $expected = "default-src *; script-src *; object-src *; style-src *; img-src *; media-src *; frame-src *; frame-ancestors *; child-src *; font-src *; connect-src *; base-uri *; report-uri " . base_path() . $this->reportPath . '; upgrade-insecure-requests';
     $this->assertSession()->responseHeaderEquals('Content-Security-Policy', $expected);
     $this->assertSession()->responseHeaderEquals('X-Content-Security-Policy', NULL);
     $this->assertSession()->responseHeaderEquals('X-WebKit-CSP', NULL);
@@ -155,11 +157,12 @@ class SecKitTestCaseTest extends BrowserTestBase {
       'seckit_xss[csp][child-src]' => '*',
       'seckit_xss[csp][font-src]' => '*',
       'seckit_xss[csp][connect-src]' => '*',
+      'seckit_xss[csp][base-uri]' => '*',
       'seckit_xss[csp][report-uri]' => $this->reportPath,
       'seckit_xss[csp][upgrade-req]' => TRUE,
     ];
     $this->drupalPostForm('admin/config/system/seckit', $form, t('Save configuration'));
-    $expected = 'default-src *; script-src *; object-src *; style-src *; img-src *; media-src *; frame-src *; frame-ancestors *; child-src *; font-src *; connect-src *; report-uri ' . base_path() . $this->reportPath . '; upgrade-insecure-requests';
+    $expected = "default-src *; script-src *; object-src *; style-src *; img-src *; media-src *; frame-src *; frame-ancestors *; child-src *; font-src *; connect-src *; base-uri *; report-uri " . base_path() . $this->reportPath . '; upgrade-insecure-requests';
     $this->assertSession()->responseHeaderEquals('Content-Security-Policy', $expected);
     $this->assertSession()->responseHeaderEquals('X-Content-Security-Policy', $expected);
     $this->assertSession()->responseHeaderEquals('X-WebKit-CSP', NULL);
@@ -184,11 +187,12 @@ class SecKitTestCaseTest extends BrowserTestBase {
       'seckit_xss[csp][child-src]' => '*',
       'seckit_xss[csp][font-src]' => '*',
       'seckit_xss[csp][connect-src]' => '*',
+      'seckit_xss[csp][base-uri]' => '*',
       'seckit_xss[csp][report-uri]' => $this->reportPath,
       'seckit_xss[csp][upgrade-req]' => TRUE,
     ];
     $this->drupalPostForm('admin/config/system/seckit', $form, t('Save configuration'));
-    $expected = 'default-src *; script-src *; object-src *; style-src *; img-src *; media-src *; frame-src *; frame-ancestors *; child-src *; font-src *; connect-src *; report-uri ' . base_path() . $this->reportPath . '; upgrade-insecure-requests';
+    $expected = "default-src *; script-src *; object-src *; style-src *; img-src *; media-src *; frame-src *; frame-ancestors *; child-src *; font-src *; connect-src *; base-uri *; report-uri " . base_path() . $this->reportPath . '; upgrade-insecure-requests';
     $this->assertSession()->responseHeaderEquals('Content-Security-Policy', $expected);
     $this->assertSession()->responseHeaderEquals('X-Content-Security-Policy', NULL);
     $this->assertSession()->responseHeaderEquals('X-WebKit-CSP', $expected);
@@ -247,12 +252,13 @@ class SecKitTestCaseTest extends BrowserTestBase {
       'seckit_xss[csp][child-src]' => '',
       'seckit_xss[csp][font-src]' => '',
       'seckit_xss[csp][connect-src]' => '',
+      'seckit_xss[csp][base-uri]' => "'self'",
       'seckit_xss[csp][report-uri]' => $this->reportPath,
       'seckit_xss[csp][upgrade-req]' => FALSE,
       'seckit_xss[csp][policy-uri]' => '',
     ];
     $this->drupalPostForm('admin/config/system/seckit', $form, t('Save configuration'));
-    $expected = "default-src self; report-uri " . base_path() . $this->reportPath;
+    $expected = "default-src self; base-uri 'self'; report-uri " . base_path() . $this->reportPath;
     $this->assertSession()->responseHeaderEquals('Content-Security-Policy', $expected);
     $this->assertSession()->responseHeaderEquals('X-Content-Security-Policy', $expected);
     $this->assertSession()->responseHeaderEquals('X-WebKit-CSP', $expected);
@@ -320,15 +326,16 @@ class SecKitTestCaseTest extends BrowserTestBase {
       $form['seckit_xss[csp][vendor-prefix][x]'] = TRUE;
       $form['seckit_xss[csp][vendor-prefix][webkit]'] = TRUE;
       $form['seckit_xss[csp][default-src]'] = 'self';
+      $form['seckit_xss[csp][base-uri]'] = "'self'";
       $form['seckit_xss[csp][report-uri]'] = $report_uri['uri'];
       $this->drupalPostForm('admin/config/system/seckit', $form, t('Save configuration'));
       if ($report_uri['valid']) {
         $base_path = ($report_uri['absolute']) ? '' : base_path();
-        $expected = 'default-src self; report-uri ' . $base_path . $report_uri['uri'];
+        $expected = "default-src self; base-uri 'self'; report-uri " . $base_path . $report_uri['uri'];
         if (!$report_uri['absolute'] && strpos($report_uri['uri'], '/') === 0) {
           // In this case, check that the leading slash on the relative path
           // was not mistakenly turned into two leading slashes.
-          $expected = 'default-src self; report-uri ' . $base_path . ltrim($report_uri['uri'], '/');
+          $expected = "default-src self; base-uri 'self'; report-uri " . $base_path . ltrim($report_uri['uri'], '/');
         }
         $this->assertSession()->responseHeaderEquals('Content-Security-Policy', $expected);
         $this->assertSession()->responseHeaderEquals('X-Content-Security-Policy', $expected);

2.x branch in the fork isn't up to date, I think, so am uploading an updated patch instead.

Found a typo and updated to include changes to the JS that Adds/removes attributes.

The patch from #35 is working on Drupal 10.2.2 and Seckit 2.0.1.

For 2.0.3

Thanks @leo-liao,
New patch applies cleanly to 2.x

I was going to add the patch to an MR, but I discovered a failing test and a Syntax error, which are now fixed.

A bit new code, this should probably be reviewed again.