Skip to content

fix(ui): disable critical-inlining to prevent CSP failure#893

Merged
imnotjames merged 1 commit into
grimmory-tools:developfrom
imnotjames:fix/-/avoid-production-csp-issue
Apr 26, 2026
Merged

fix(ui): disable critical-inlining to prevent CSP failure#893
imnotjames merged 1 commit into
grimmory-tools:developfrom
imnotjames:fix/-/avoid-production-csp-issue

Conversation

@imnotjames

@imnotjames imnotjames commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Description

this critical inlining option is meant to improve the first contentful paint metric but does so in a way that is not compatible with a safe content-security-policy

this only affected production builds which is why we never saw it in development

Changes

disalble angular option projects.grimmory.architect.build.configurations.production.optimization.styles.inlineCritical

Summary by CodeRabbit

  • Chores
    • Optimized production build configuration for improved app performance and reduced bundle sizes.

this critical inlining option is meant to improve the first contentful
paint metric but does so in a way that is not compatible with a safe
content-security-policy

this only affected production builds which is why we never saw it in
development
@coderabbitai

coderabbitai Bot commented Apr 26, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The production build configuration in Angular's configuration file now explicitly defines optimization settings: script optimization enabled, style minification enabled with critical CSS inlining disabled, and font optimization enabled. The service worker configuration remains unchanged.

Changes

Cohort / File(s) Summary
Angular Build Configuration
frontend/angular.json
Production build now includes explicit optimization settings for scripts, styles (without critical CSS inlining), and fonts.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

frontend, enhancement

Poem

🐰 Scripts optimized, styles so fine,
Fonts dance lighter without a line,
CSS critical takes a rest,
Angular builds now do their best!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title follows the conventional commit format with type 'fix' and scope 'ui', accurately summarizing the main change of disabling critical CSS inlining.
Description check ✅ Passed The description includes required sections (Description and Changes) and clearly explains the issue with critical inlining and CSP incompatibility, though the specific option path contains a typo ('disalble' instead of 'disable').
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
frontend/angular.json (1)

99-106: Simplify optimization config to show only the non-default override.

Angular's production optimization defaults apply scripts: true, styles.minify: true, and fonts: true. Only inlineCritical: false is an explicit override; remove the redundant defaults to make the intent clearer and reduce future diff noise.

♻️ Suggested minimal form
       "outputHashing": "all",
       "serviceWorker": "ngsw-config.json",
       "optimization": {
-        "scripts": true,
         "styles": {
-          "minify": true,
           "inlineCritical": false
-        },
-        "fonts": true
+        }
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/angular.json` around lines 99 - 106, The optimization block contains
redundant defaults; simplify it by removing explicit "scripts": true, "styles":
{ "minify": true }, and "fonts": true entries and keep only the actual override
"inlineCritical": false under the "optimization" -> "styles" object so the
config shows only the non-default change (refer to the "optimization",
"scripts", "styles", "minify", "inlineCritical", and "fonts" keys in the
existing snippet).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@frontend/angular.json`:
- Around line 99-106: The optimization block contains redundant defaults;
simplify it by removing explicit "scripts": true, "styles": { "minify": true },
and "fonts": true entries and keep only the actual override "inlineCritical":
false under the "optimization" -> "styles" object so the config shows only the
non-default change (refer to the "optimization", "scripts", "styles", "minify",
"inlineCritical", and "fonts" keys in the existing snippet).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 1e3555ab-3853-4ce1-9034-f8260e00ee5a

📥 Commits

Reviewing files that changed from the base of the PR and between 79455ff and 9cb0f1b.

📒 Files selected for processing (1)
  • frontend/angular.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Test Suite / Backend Tests
  • GitHub Check: Test Suite / Frontend Tests
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (actions)
  • GitHub Check: Analyze (java-kotlin)
  • GitHub Check: Frontend Lint Threshold Check
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 848
File: frontend/src/index.html:9-9
Timestamp: 2026-04-25T02:36:26.141Z
Learning: In the grimmory project (`frontend/src/index.html`), adding broad CSP directives like `default-src 'self'` with targeted overrides breaks the application. The CSP must be kept minimal and targeted to the actual resource needs of the app. Avoid recommending broad defense-in-depth CSP additions for this project.
🔇 Additional comments (1)
frontend/angular.json (1)

99-106: Targeted fix is correct.

Disabling inlineCritical is the right way to avoid emitting inline <style> blocks that would otherwise be blocked by a strict style-src 'self' CSP without a nonce/hash. The development configuration is unaffected (still optimization: false), matching the PR description that the issue only manifests in production.

@imnotjames imnotjames merged commit b0c8572 into grimmory-tools:develop Apr 26, 2026
16 checks passed
@imnotjames imnotjames deleted the fix/-/avoid-production-csp-issue branch April 26, 2026 01:54
dsmouse pushed a commit to dsmouse/grimmory that referenced this pull request May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants