From 6a97f7c20e093182d522d1eff88086bcd31bd4ff Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 4 Mar 2026 14:39:24 +0200 Subject: [PATCH] Bug 38365: Make swagger-cli happy Schema type [array,object] is not valid in OpenAPI version 2. There are two different CSP reporting directives. One is the now deprecated "report-uri" directive. Browsers typically send a single JSON object following the CSP2 specification. "report-uri" is deprecated in CSP3 in favor of the newer "report-to" directive. It is expected to contain an array of report objects. There are browser that do not yet support the "report-to" directive, so both directives should be supported. There are also variations among the "report-uri" format. See https://www.tollmanz.com/content-security-policy-report-samples/ Because of these variations, it becomes quite complex to handle the most common cases in a single API endpoint. We could build two separate endpoints for both directives. Or we can accept any type of input and handle the variations in a single endpoint. This patch attempts to do that. Before this patch input parameters were validated by JSON::Validator. That is the normal and recommended way to build OpenAPI endpoints in Koha. After this patch we remove all input parameter restrictions from the OpenAPI spec, and simply accept any type of body. This means the controller will now always return a HTTP 204 regardless of the type of data passed to the controller. It instead truncates too long input before writing them into log files. As this is not an endpoint that developers will build new Koha clients for, we do not have to offer a very in-depth input parameter spec for this endpoint. To test: 1. t/db_dependent/api/v1/public/csp_reports.t --- Koha/REST/V1/CSPReports.pm | 7 ++ api/v1/swagger/definitions/csp_report.yaml | 60 ----------- .../swagger/definitions/csp_report_body.yaml | 56 ---------- .../definitions/csp_report_uri_to.yaml | 27 ----- api/v1/swagger/paths/public_csp_reports.yaml | 7 -- api/v1/swagger/swagger.yaml | 4 - t/db_dependent/api/v1/public/csp_reports.t | 102 ++++++++++++------ 7 files changed, 76 insertions(+), 187 deletions(-) delete mode 100644 api/v1/swagger/definitions/csp_report.yaml delete mode 100644 api/v1/swagger/definitions/csp_report_body.yaml delete mode 100644 api/v1/swagger/definitions/csp_report_uri_to.yaml diff --git a/Koha/REST/V1/CSPReports.pm b/Koha/REST/V1/CSPReports.pm index 81676344f29..e1e89f542b8 100644 --- a/Koha/REST/V1/CSPReports.pm +++ b/Koha/REST/V1/CSPReports.pm @@ -81,6 +81,13 @@ sub add { my $line_number = $csp_report->{'lineNumber'} // $csp_report->{'line-number'} // ''; my $column_number = $csp_report->{'columnNumber'} // $csp_report->{'column-number'} // ''; + $document_uri = length($document_uri) > 200 ? substr( $document_uri, 0, 200 ) . "..." : $document_uri; + $violated_dir = length($violated_dir) > 100 ? substr( $violated_dir, 0, 100 ) . "..." : $violated_dir; + $blocked_uri = length($blocked_uri) > 200 ? substr( $blocked_uri, 0, 200 ) . "..." : $blocked_uri; + $source_file = length($source_file) > 200 ? substr( $source_file, 0, 200 ) . "..." : $source_file; + $line_number = length($line_number) > 7 ? substr( $line_number, 0, 7 ) . "..." : $line_number; + $column_number = length($column_number) > 7 ? substr( $column_number, 0, 7 ) . "..." : $column_number; + # Build location string if available my $location = ''; if ($source_file) { diff --git a/api/v1/swagger/definitions/csp_report.yaml b/api/v1/swagger/definitions/csp_report.yaml deleted file mode 100644 index 064ff711ac6..00000000000 --- a/api/v1/swagger/definitions/csp_report.yaml +++ /dev/null @@ -1,60 +0,0 @@ ---- -type: object -description: | - A Content-Security-Policy violation report as sent by browsers. - See https://www.w3.org/TR/CSP3/#violation-reports for specification. -properties: - document-uri: - type: string - description: The URI of the document where the violation occurred - maxLength: 200 - referrer: - type: string - description: The referrer of the document where the violation occurred - maxLength: 200 - violated-directive: - type: string - description: The directive that was violated (e.g., "script-src 'self'") - maxLength: 100 - effective-directive: - type: string - description: The effective directive that was violated - maxLength: 100 - original-policy: - type: string - description: The original CSP policy as specified in the header - maxLength: 400 - disposition: - type: string - description: Either "enforce" or "report" depending on CSP mode - enum: - - enforce - - report - blocked-uri: - type: string - description: The URI of the resource that was blocked - maxLength: 200 - line-number: - type: integer - description: Line number in the document where the violation occurred - minimum: 0 - maximum: 999999 - column-number: - type: integer - description: Column number where the violation occurred - minimum: 0 - maximum: 999999 - source-file: - type: string - description: The URI of the script where the violation occurred - maxLength: 200 - status-code: - type: integer - description: HTTP status code of the document - minimum: 0 - maximum: 999 - script-sample: - type: string - description: First 40 characters of the inline script that caused the violation - maxLength: 41 -additionalProperties: true diff --git a/api/v1/swagger/definitions/csp_report_body.yaml b/api/v1/swagger/definitions/csp_report_body.yaml deleted file mode 100644 index 397bbb5dca7..00000000000 --- a/api/v1/swagger/definitions/csp_report_body.yaml +++ /dev/null @@ -1,56 +0,0 @@ ---- -type: object -description: | - A Content-Security-Policy violation report as sent by browsers. - See https://www.w3.org/TR/CSP3/#violation-reports for specification. -properties: - documentURL: - type: string - description: The URI of the document where the violation occurred - maxLength: 200 - referrer: - type: string - description: The referrer of the document where the violation occurred - maxLength: 200 - effectiveDirective: - type: string - description: The effective directive that was violated - maxLength: 100 - originalPolicy: - type: string - description: The original CSP policy as specified in the header - maxLength: 400 - disposition: - type: string - description: Either "enforce" or "report" depending on CSP mode - enum: - - enforce - - report - blockedURL: - type: string - description: The URI of the resource that was blocked - maxLength: 200 - lineNumber: - type: integer - description: Line number in the document where the violation occurred - minimum: 0 - maximum: 999999 - columnNumber: - type: integer - description: Column number where the violation occurred - maximum: 999999 - minimum: 0 - sample: - type: string - description: First 40 characters of the inline script that caused the violation - maxLength: 41 - sourceFile: - type: string - description: The URI of the script where the violation occurred - maxLength: 200 - statusCode: - type: integer - description: HTTP status code of the document - minimum: 0 - maximum: 999 -additionalProperties: true diff --git a/api/v1/swagger/definitions/csp_report_uri_to.yaml b/api/v1/swagger/definitions/csp_report_uri_to.yaml deleted file mode 100644 index 513f915b41a..00000000000 --- a/api/v1/swagger/definitions/csp_report_uri_to.yaml +++ /dev/null @@ -1,27 +0,0 @@ ---- -type: - - array - - object -description: | - A Content-Security-Policy violation report as sent by browsers. - See https://www.w3.org/TR/CSP3/#violation-reports for specification. -properties: - csp-report: - $ref: "csp_report.yaml" - age: - type: integer - minimum: 0 - maximum: 9999999999 - body: - $ref: "csp_report_body.yaml" - user_agent: - type: string - maxLength: 300 - url: - type: string - maxLength: 200 - type: - type: string - enum: - - "csp-violation" -additionalProperties: true diff --git a/api/v1/swagger/paths/public_csp_reports.yaml b/api/v1/swagger/paths/public_csp_reports.yaml index 6418d2262b7..2af72fddfbf 100644 --- a/api/v1/swagger/paths/public_csp_reports.yaml +++ b/api/v1/swagger/paths/public_csp_reports.yaml @@ -20,13 +20,6 @@ - application/reports+json produces: - application/json - parameters: - - name: body - in: body - description: CSP violation report - required: true - schema: - $ref: "../swagger.yaml#/definitions/csp_report_uri_to" responses: "204": description: Report received successfully diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index 9988c24e3fd..54daf2869ae 100644 --- a/api/v1/swagger/swagger.yaml +++ b/api/v1/swagger/swagger.yaml @@ -42,10 +42,6 @@ definitions: $ref: ./definitions/circ-rule-kind.yaml city: $ref: ./definitions/city.yaml - csp_report: - $ref: ./definitions/csp_report.yaml - csp_report_uri_to: - $ref: ./definitions/csp_report_uri_to.yaml credit: $ref: ./definitions/credit.yaml debit: diff --git a/t/db_dependent/api/v1/public/csp_reports.t b/t/db_dependent/api/v1/public/csp_reports.t index 9928d723d49..742c238ab4c 100755 --- a/t/db_dependent/api/v1/public/csp_reports.t +++ b/t/db_dependent/api/v1/public/csp_reports.t @@ -42,7 +42,7 @@ subtest 'add() tests' => sub { foreach my $csp_report_type ( 'report-uri', 'report-to' ) { subtest "Test $csp_report_type" => sub { - plan tests => 17; + plan tests => 9; my $content_type = $csp_report_type eq 'report-to' ? 'application/csp-report' : 'application/reports+json'; # Test with standard CSP report-uri format @@ -69,37 +69,6 @@ subtest 'add() tests' => sub { _convert_report_uri_to_report_to( $csp_param_names, $csp_report ) if $csp_report_type eq 'report-to'; my $csp_report_body_key = $csp_report_type eq 'report-uri' ? 'csp-report' : 'body'; - $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'line-number'} } = 99999999999999999; - - # Too large integers should be rejected - $t->post_ok( '/api/v1/public/csp-reports' => { 'Content-Type' => $content_type } => json => $csp_report ) - ->status_is( 400, 'CSP report rejected (400) because of line-number exceeding maximum value' ); - - $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'line-number'} } = -1; - - # Too small integers should be rejected - $t->post_ok( '/api/v1/public/csp-reports' => { 'Content-Type' => $content_type } => json => $csp_report ) - ->status_is( 400, 'CSP report rejected (400) because of line-number not reaching minimum value' ); - $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'line-number'} } = 42; - - $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'disposition'} } = 'this is not okay'; - - # Enum values should be confirmed - $t->post_ok( '/api/v1/public/csp-reports' => { 'Content-Type' => $content_type } => json => $csp_report ) - ->status_is( - 400, - 'CSP report rejected (400) because of disposition is not either "enforce" nor "report"' - ); - $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'disposition'} } = 'enforce'; - - $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'script-sample'} } = - 'this is way too long script sample. a maximum of only 40 characters is allowed'; - - # Too long strings should be rejected - $t->post_ok( '/api/v1/public/csp-reports' => { 'Content-Type' => $content_type } => json => $csp_report ) - ->status_is( 400, 'CSP report rejected (400) because of script-sample exceeding maximum length' ); - $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'script-sample'} } = 'console.log("hi");'; - # Anonymous request should work (browsers send these without auth) $t->post_ok( '/api/v1/public/csp-reports' => { 'Content-Type' => $content_type } => json => $csp_report ) ->status_is( 204, 'CSP report accepted' ); @@ -135,7 +104,7 @@ subtest 'add() tests' => sub { ->status_is( 204, 'Minimal CSP report accepted' ); subtest 'make sure log entries are being written' => sub { - plan tests => 22; + plan tests => 30; my $log4perl_conf_file = C4::Context->config('intranetdir') . '/etc/log4perl.conf'; open my $fh, '<:encoding(UTF-8)', $log4perl_conf_file or do { @@ -213,6 +182,73 @@ subtest 'add() tests' => sub { $appender->clear(); $appenderplack->clear(); + # Do not crash if input is not JSON + $t->post_ok( '/api/v1/public/csp-reports' => { 'Content-Type' => $content_type } => json => "not json" ) + ->status_is( 204, 'CSP report accepted' ); + + # Too large integers should be truncated + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'line-number'} } = 99999999999999999; + $t->post_ok( + '/api/v1/public/csp-reports' => { 'Content-Type' => $content_type } => json => $csp_report ) + ->status_is(204); + $expected_log_entry = sprintf( + "CSP violation: '%s' blocked '%s' on page '%s'%s", + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'violated-directive'} }, + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'blocked-uri'} }, + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'document-uri'} }, + ' at ' + . $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'source-file'} } . ':' + . '9999999...:' + . $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'column-number'} } + ); + + like( + $appender->buffer, qr/$expected_log_entry/ + , 'Too long line number was truncated' + ); + $appender->clear(); + + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'line-number'} } = 42; + + # Too long strings should be rejected + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'effective-directive'} } = + 'this is an example of a way too long effective-directive. a maximum of only 100 characters is allowed. so this should be truncated'; + $t->post_ok( + '/api/v1/public/csp-reports' => { 'Content-Type' => $content_type } => json => $csp_report ) + ->status_is(204); + + $expected_log_entry = sprintf( + "CSP violation: '%s' blocked '%s' on page '%s'%s", + substr( + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'effective-directive'} }, 0, 100 + ) + . '...', + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'blocked-uri'} }, + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'document-uri'} }, + ' at ' + . $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'source-file'} } . ':' + . $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'line-number'} } . ':' + . $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'column-number'} } + ); + + like( + $appender->buffer, qr/$expected_log_entry/ + , 'Too long line number was truncated' + ); + $appender->clear(); + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'effective-directive'} } = 'script-src'; + + $expected_log_entry = sprintf( + "CSP violation: '%s' blocked '%s' on page '%s'%s", + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'violated-directive'} }, + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'blocked-uri'} }, + $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'document-uri'} }, + ' at ' + . $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'source-file'} } . ':' + . $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'line-number'} } . ':' + . $csp_report->{$csp_report_body_key}->{ $csp_param_names->{'column-number'} } + ); + subtest 'test array as input' => sub { plan tests => 7; -- 2.34.1