Skip to content

⚡ Optimize Sanitiser schema merging #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Snider wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

⚡ Optimize Sanitiser schema merging #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
273485d
Optimize Sanitiser by pre-calculating effective schemas
Snider Feb 2, 2026
6daa552
Fix CI Psalm output corruption
Snider Feb 2, 2026
7d64069
Fix CI Psalm SARIF upload validation error
Snider Feb 2, 2026
5e8da3f
Fix Psalm SARIF validation errors using jq
Snider Feb 2, 2026
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
12 changes: 9 additions & 3 deletions .github/workflows/qa.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,17 @@ jobs:
id: psalm
run: |
set +e
vendor/bin/psalm --output-format=json --show-info=false > psalm.json 2>&1
# Don't redirect stderr to stdout to avoid corrupting JSON output
vendor/bin/psalm --output-format=json --show-info=false > psalm.json
EXIT_CODE=$?

# Generate SARIF for GitHub Security
vendor/bin/psalm --output-format=sarif --show-info=false > psalm.sarif 2>&1 || true
vendor/bin/psalm --output-format=sarif --show-info=false > psalm.sarif || true

# Fix invalid SARIF generated by Psalm (locations with 0)
if [ -f psalm.sarif ]; then
jq 'walk(if type == "object" and .region? then .region |= with_entries(if (.key | test("Line|Column")) and .value < 1 then .value = 1 else . end) else . end)' psalm.sarif > psalm.sarif.tmp && mv psalm.sarif.tmp psalm.sarif
fi
Comment on lines +191 to +194
Copy link

@coderabbitai coderabbitai bot Feb 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add error handling for the SARIF normalization step.

The jq command correctly normalizes invalid SARIF locations, but lacks error handling. If jq fails (e.g., malformed JSON from Psalm), the mv will overwrite psalm.sarif with an empty or corrupt file.

Proposed fix with error handling 
           # Fix invalid SARIF generated by Psalm (locations with 0)
           if [ -f psalm.sarif ]; then
-            jq 'walk(if type == "object" and .region? then .region |= with_entries(if (.key | test("Line|Column")) and .value < 1 then .value = 1 else . end) else . end)' psalm.sarif > psalm.sarif.tmp && mv psalm.sarif.tmp psalm.sarif
+            if jq 'walk(if type == "object" and .region? then .region |= with_entries(if (.key | test("Line|Column")) and .value < 1 then .value = 1 else . end) else . end)' psalm.sarif > psalm.sarif.tmp; then
+              mv psalm.sarif.tmp psalm.sarif
+            else
+              echo "Warning: Failed to normalize SARIF, using original"
+              rm -f psalm.sarif.tmp
+            fi
           fi
🤖 Prompt for AI Agents 
In @.github/workflows/qa.yml around lines 191 - 194, The SARIF normalization
uses jq to write to psalm.sarif.tmp and then mv overwrites psalm.sarif even if
jq fails; change the step around the jq invocation (the jq 'walk(...)' command
targeting psalm.sarif and producing psalm.sarif.tmp) so you only mv
psalm.sarif.tmp -> psalm.sarif when jq exits successfully, and on jq failure
delete the temp file, emit an error message (e.g., echo to stderr) and exit
non‑zero to avoid clobbering psalm.sarif.


ERRORS=$(jq 'length' psalm.json 2>/dev/null || echo "0")
echo "errors=${ERRORS}" >> $GITHUB_OUTPUT
Expand All @@ -200,7 +206,7 @@ jobs:

- name: Upload Psalm SARIF
if: always()
uses: github/codeql-action/upload-sarif@v3
uses: github/codeql-action/upload-sarif@v4
with:
sarif_file: psalm.sarif
category: psalm
Expand Down
32 changes: 32 additions & 0 deletions bad_sarif.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"runs": [
{
"results": [
{
"locations": [
{
"physicalLocation": {
"region": {
"startLine": 0,
"startColumn": 0,
"endLine": 0,
"endColumn": 0
}
}
},
{
"physicalLocation": {
"region": {
"startLine": 10,
"startColumn": 5,
"endLine": 10,
"endColumn": 15
}
}
}
]
}
]
}
]
}
Empty file added psalm.json
View file
Open in desktop
Empty file.
3 changes: 3 additions & 0 deletions psalm.log
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Psalm has detected issues in your platform:

Psalm requires a PHP version ">= 8.3.16". You are running 8.3.6.
3 changes: 3 additions & 0 deletions psalm.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Psalm has detected issues in your platform:

Psalm requires a PHP version ">= 8.3.16". You are running 8.3.6.
Comment on lines +1 to +3
Copy link

@coderabbitai coderabbitai bot Feb 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove accidentally committed runtime artifact.

This file appears to be actual Psalm stderr output captured during development/debugging, not a test fixture. It should not be committed to the repository as it:

  1. Contains environment-specific runtime information
  2. Will become stale immediately
  3. Serves no purpose in the codebase

Additionally, this reveals that the CI environment's PHP version (8.3.6) doesn't meet Psalm's requirement (>= 8.3.16), which may affect analysis accuracy.

Consider adding this file to .gitignore and updating the workflow's PHP version to meet Psalm's requirements:

 env:
-  PHP_VERSION: '8.3'
+  PHP_VERSION: '8.3.16'
🤖 Prompt for AI Agents 
In `@psalm.stderr` around lines 1 - 3, Remove the committed runtime artifact
"psalm.stderr" from the repository, add "psalm.stderr" to .gitignore to prevent
future commits, and update the project's CI workflow PHP version setting to at
least 8.3.16 so Psalm runs against a compatible PHP version; ensure the CI job
that installs/sets up PHP (the workflow step that selects the PHP runtime) is
updated accordingly and re-run CI to verify Psalm no longer reports the version
mismatch.

Empty file added psalm.stdout
View file
Open in desktop
Empty file.
Empty file added psalm_clean.json
View file
Open in desktop
Empty file.
Empty file added psalm_verify.json
View file
Open in desktop
Empty file.
Empty file added psalm_verify_null.json
View file
Open in desktop
Empty file.
38 changes: 33 additions & 5 deletions src/Core/Input/Sanitiser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ class Sanitiser
*/
protected array $schema = [];

/**
* Cache of pre-merged effective schemas for performance.
*
* @var array<string, array>
*/
protected array $preparedSchema = [];

/**
* Optional logger for audit logging.
*/
Expand Down Expand Up @@ -207,6 +214,8 @@ public function __construct(
$this->normalizeUnicode = $normalizeUnicode;
$this->maxLength = $maxLength;
$this->allowedHtmlTags = $allowedHtmlTags;

$this->prepareSchema();
}

/**
Expand All @@ -230,6 +239,7 @@ public function withSchema(array $schema): static
{
$clone = clone $this;
$clone->schema = $schema;
$clone->prepareSchema();

return $clone;
}
Expand Down Expand Up @@ -449,9 +459,31 @@ protected function applyPresetToFields(string $presetName, array $fields): stati
}
}

$clone->prepareSchema();

return $clone;
}

/**
* Prepare effective schemas for all fields.
*/
protected function prepareSchema(): void
{
$globalSchema = $this->schema['*'] ?? [];
$this->preparedSchema = [];

// Pre-calculate for known fields
foreach ($this->schema as $field => $fieldSchema) {
if ($field === '*') {
continue;
}
$this->preparedSchema[$field] = array_merge($globalSchema, $fieldSchema);
}

// Store global schema for fallback
$this->preparedSchema['*'] = $globalSchema;
}

// =========================================================================
// TRANSFORMATION HOOKS
// =========================================================================
Expand Down Expand Up @@ -688,11 +720,7 @@ protected function filterRecursive(array $input, string $path = ''): array
protected function filterString(string $value, string $path, string $fieldName): string
{
$original = $value;
$fieldSchema = $this->schema[$fieldName] ?? [];
$globalSchema = $this->schema['*'] ?? [];

// Merge global schema with field-specific schema (field takes precedence)
$effectiveSchema = array_merge($globalSchema, $fieldSchema);
$effectiveSchema = $this->preparedSchema[$fieldName] ?? $this->preparedSchema['*'];

// Step 0: Apply before hooks
$value = $this->applyBeforeHooks($value, $fieldName);
Expand Down
32 changes: 32 additions & 0 deletions test.sarif
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"runs": [
{
"results": [
{
"locations": [
{
"physicalLocation": {
"region": {
"startLine": 0,
"startColumn": 0,
"endLine": 0,
"endColumn": 0
}
}
},
{
"physicalLocation": {
"region": {
"startLine": 10,
"startColumn": 5,
"endLine": 10,
"endColumn": 15
}
}
}
]
}
]
}
]
}
32 changes: 32 additions & 0 deletions test_clean.sarif
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"runs": [
{
"results": [
{
"locations": [
{
"physicalLocation": {
"region": {
"startLine": 1,
"startColumn": 1,
"endLine": 1,
"endColumn": 1
}
}
},
{
"physicalLocation": {
"region": {
"startLine": 10,
"startColumn": 5,
"endLine": 10,
"endColumn": 15
}
}
}
]
}
]
}
]
}
Loading