[jf-cbd]

Base information

Question	Answer
Related to a SourceForge thread / Another PR / A GitHub Issue / Combodo ticket?	9477
Type of change?	Enhancement

Symptom (bug) / Objective (enhancement)

Add unit test to ensure sandboxing is present, add some more default-src info, remove old code

Reproduction procedure (enhancement - if needed)

On iTop 3.2.3, create a DocumentFile, upload a picture and open it. See in the network console that the request have CSP sandboxing as well as "default-src 'none' " instruction.

Proposed solution (bug and enhancement)

Add more header details, add unit test, to avoid regression, clean code.

Checklist before requesting a review

• If my PR is a bug fix or an enhancement, I opened an issue that got reviewed and approved by Combodo
• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• I have added a unit test, otherwise I have explained why I couldn't
• Is the PR clear and detailed enough so anyone can understand without digging in the code?

[Copilot]

Copilot review overview

🟡 Changes recommended

The new test uses an undefined variable and does not verify the newly added CSP directive.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Define $bDisable explicitly before passing it to Config::Set() · New
• Assert CSP includes the new default-src 'none' directive · New

What changed in this PR

Improves inline document security headers and removes obsolete XSS filtering.

Changes:

• Adds default-src 'none' to the document CSP.
• Removes legacy script filtering.
• Adds a regression test for document response headers.

File	Description
tests/​php-unit-tests/​unitary-tests/​pages/​AjaxRenderTest.php	Tests document CSP headers.
sources/​Application/​WebPage/​DownloadPage.php	Outputs document content directly.
sources/​Application/​WebPage/​AjaxPage.php	Removes legacy XSS filter.
pages/​ajax.render.php	Strengthens inline-document CSP.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[greptile-apps]

Confidence Score: 5/5

[Critical risk] Changes how inline documents are sandboxed against script execution.

The PR appears safe to merge based on the changes since the previous review and the resolved prior finding.

Summary

The PR adds default-src 'none' to the inline-document sandbox policy, removes legacy script filtering from download output, and adds a sandbox-header test. Since the previous review, it has corrected the test’s undefined variable and made whitespace-only changes to download output.

_{Reviews (2) · Last reviewed commit: "Fix test"}

[Molkobain]

@jf-cbd please mind to name the PR correctly, not straight out of the branch name. (Among other considerations, it won't appear in the corresponding ticket once merged) :)

[Copilot]

Copilot review overview

🟢 Approval recommended

The focused security-header enhancement and cleanup are consistent with the document delivery flow and covered by a regression test.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Define $bDisable explicitly before passing it to Config::Set()
• Assert CSP includes the new default-src 'none' directive