Skip to content

Security: Protocol-relative third-party stylesheet URLs allow insecure dependency loading#9917

Closed
tomaioo wants to merge 1 commit into
neomjs:devfrom
tomaioo:fix/security/protocol-relative-third-party-stylesheet
Closed

Security: Protocol-relative third-party stylesheet URLs allow insecure dependency loading#9917
tomaioo wants to merge 1 commit into
neomjs:devfrom
tomaioo:fix/security/protocol-relative-third-party-stylesheet

Conversation

@tomaioo

@tomaioo tomaioo commented Apr 12, 2026

Copy link
Copy Markdown

Summary

Security: Protocol-relative third-party stylesheet URLs allow insecure dependency loading

Problem

Severity: Medium | File: apps/realworld/index.html:L6

The page loads external CSS via protocol-relative URLs (//...). If the page is ever served over HTTP (or behind a misconfigured proxy), these dependencies can be fetched insecurely and altered by a man-in-the-middle, enabling content/script injection through CSS or compromised assets.

Solution

Use explicit https:// URLs for all third-party resources, enforce HSTS on the hosting domain, and avoid protocol-relative URLs.

Changes

  • apps/realworld/index.html (modified)

The page loads external CSS via protocol-relative URLs (`//...`). If the page is ever served over HTTP (or behind a misconfigured proxy), these dependencies can be fetched insecurely and altered by a man-in-the-middle, enabling content/script injection through CSS or compromised assets.

Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
@neo-gemini-pro

Copy link
Copy Markdown
Contributor

Input from Gemini 3.1 Pro:

✦ # PR Review Summary

Status: Approved

Peer-Review Opening: Thanks for putting this together and catching this! Using explicit HTTPS URLs is a much better security posture and ensures dependencies resolve correctly regardless of the hosting environment.


🕸️ Context & Graph Linking


🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

I actively looked for any other instances of protocol-relative URLs (//...) in apps/realworld/index.html and other core templates and found no concerns. The changes cleanly target the requested dependencies.

Rhetorical-Drift Audit (per guide §7.4):
N/A


🧠 Graph Ingestion Notes

  • [RETROSPECTIVE]: Replacing protocol-relative URLs with explicit https:// is a small but critical security hygiene step to prevent insecure dependency loading.

🛂 Provenance Audit

N/A


🎯 Close-Target Audit

N/A


📡 MCP-Tool-Description Budget Audit

N/A


🔌 Wire-Format Compatibility Audit

N/A


🔗 Cross-Skill Integration Audit

N/A


🧪 Test-Execution Audit

  • Branch checked out locally
  • If a test file changed: Ran the specific test file.
  • If code changed: Verified if there are tests, or if new tests are needed.

Findings: No tests needed (template/HTML change only).


📋 Required Actions

No required actions — eligible for human merge.


📊 Evaluation Metrics

  • [ARCH_ALIGNMENT]: 100 - I actively considered Neo's layout, routing, and component architecture and confirmed none apply. This is a pure HTML change.
  • [CONTENT_COMPLETENESS]: 100 - I actively considered JSDoc and Anchor & Echo requirements and confirmed none apply for HTML files.
  • [EXECUTION_QUALITY]: 100 - I actively considered syntax errors, incorrect URLs, and loading issues and confirmed none apply.
  • [PRODUCTIVITY]: 100 - I actively considered the goal of the PR (fixing insecure loading) and confirmed it achieves it perfectly.
  • [IMPACT]: 30 - 70 points deducted because this is a routine security fix on a single app's index.html, not a framework-level architectural change.
  • [COMPLEXITY]: 10 - Low: Simple string replacement in an HTML file.
  • [EFFORT_PROFILE]: Quick Win - High ROI (security fix) with very low complexity.

Great catch!

@neo-gpt neo-gpt 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.

PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

  • Decision: Approve
  • Rationale: This is a narrow external security hygiene fix. The diff changes only apps/realworld/index.html and replaces protocol-relative third-party stylesheet URLs with explicit https:// URLs. Fresh validation found no residual protocol-relative href / src / url(...) entries in the touched file.

Peer-Review Opening: Thanks for the focused security cleanup. The patch is small, directly addresses the insecure dependency-loading vector described in the PR, and does not disturb the RealWorld app boot path.


🕸️ Context & Graph Linking

  • Target Epic / Issue ID: PR #9917
  • Related Graph Nodes: RealWorld app HTML shell; external CDN stylesheet loading; browser mixed-content / insecure transport hygiene

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

Documented search: I checked the exact diff and the touched file rather than relying on the PR description. Validation:

  • git diff --check origin/dev...HEAD -> clean.
  • rg -n "href=\"//|src=\"//|url\(\s*//" apps/realworld/index.html -> no matches.
  • Confirmed explicit HTTPS URLs for code.ionicframework.com, fonts.googleapis.com, and demo.productionready.io/main.css in apps/realworld/index.html.

Rhetorical-Drift Audit (per guide §7.4):

  • PR description matches the diff: protocol-relative stylesheet URLs are replaced with explicit HTTPS URLs.
  • No stale test or CI claims in the PR body.
  • Anchor & Echo / JSDoc requirements are not applicable to a static HTML shell edit.
  • Linked anchors: N/A.

Findings: Pass.


🧠 Graph Ingestion Notes

  • [KB_GAP]: None.
  • [TOOLING_GAP]: No automated checks are configured on this external PR; validation was local/static.
  • [RETROSPECTIVE]: Protocol-relative third-party assets are avoidable security debt in app entry shells; explicit HTTPS is the correct low-cost default.

🛂 Provenance Audit

N/A. No external code or architecture pattern was imported; the PR only changes URL schemes in an existing HTML file.


🎯 Close-Target Audit

N/A. This PR does not name a closing issue or epic.


📑 Contract Completeness Audit

  • Problem statement: protocol-relative third-party stylesheet URLs can resolve insecurely when served over HTTP or behind a misconfigured proxy.
  • Implementation: all three third-party stylesheet links in apps/realworld/index.html now use explicit https://.
  • Scope: no unrelated app, framework, or build behavior changed.

Findings: Pass.


🪜 Evidence Audit

  • Evidence class: L1 static validation is sufficient for a one-file HTML URL-scheme replacement.
  • No runtime behavior claim requiring browser/E2E verification was made.
  • Residual sweep against protocol-relative href / src / url(...) patterns in the touched file is clean.

Findings: Pass.


📜 Source-of-Authority Audit

N/A. The approval is based on the live diff and local validation, not on prior comment approval.


📡 MCP-Tool-Description Budget Audit

N/A. No MCP/OpenAPI tool descriptions are touched.


🔌 Wire-Format Compatibility Audit

N/A. No API, JSON, A2A, or MCP wire formats are touched.


🔗 Cross-Skill Integration Audit

N/A. No agent skills or turn-loaded substrate are touched.


🧪 Test-Execution & Location Audit

  • Branch checked out locally: fix/security/protocol-relative-third-party-stylesheet at 788e8ee22c32a7fb1b2766605d6c680f6d52229f.
  • Changed file: apps/realworld/index.html.
  • Ran format audit: git diff --check origin/dev...HEAD -> clean.
  • Ran residual URL sweep: no protocol-relative href / src / url(...) entries remain in the touched file.
  • No unit test target is applicable for this static HTML URL-scheme change.

Findings: Pass.


🛡️ CI / Security Checks Audit

  • gh pr view 9917 --json statusCheckRollup shows no automated checks configured for this PR.
  • Security posture improves by forcing HTTPS for third-party stylesheet loads.

Findings: Pass with local/static validation.


📋 Required Actions

No required actions. This PR is merge-eligible pending the human-only merge gate.


📊 Evaluation Metrics

  • [ARCH_ALIGNMENT]: 100 - Pure HTML shell hygiene; no Neo architectural surface is affected.
  • [CONTENT_COMPLETENESS]: 95 - The PR clearly describes the problem and the exact file changed; no JSDoc or ticket closure metadata applies.
  • [EXECUTION_QUALITY]: 100 - The replacement is exact and residual protocol-relative checks are clean.
  • [PRODUCTIVITY]: 100 - Fully resolves the stated insecure dependency-loading concern.
  • [IMPACT]: 30 - Low blast radius but positive security hygiene for the RealWorld app.
  • [COMPLEXITY]: 5 - One-file, three-URL static change.
  • [EFFORT_PROFILE]: Quick Win - High ROI, very low complexity.

Per AGENTS.md §0, this approval is merge eligibility only; merge execution remains human-only.

@neo-opus-ada

Copy link
Copy Markdown
Contributor

Input from Claude Opus 4.7 (Claude Code):

❋ Hi @tomaioo — late response on this, sorry for the wait. Quick context that explains why this isn't a straightforward merge:

Substrate History

Neo released GA in November 2019. apps/realworld was Neo's 2nd ever app, built specifically for the RealWorld framework comparison project (formerly known as gothinkster/realworld) — a comparison set of 100+ JS framework/library implementations of the same "Conduit" social-blog spec.

Conformance Constraint

RealWorld's acceptance criteria for entries were strict: even slight template mismatches would exclude an implementation from the comparison set. This is why Neo's apps/realworld couldn't use Neo's own component library — the canonical index.html template is the entry contract.

The Protocol-Relative URLs Are the Spec

The exact pattern in apps/realworld/index.html:L6 appears verbatim in the RealWorld frontend template specification:

<link href="//code.ionicframework.com/ionicons/2.0.1/css/ionicons.min.css" ... />
<link href="//fonts.googleapis.com/css?family=..." ... />
<link rel="stylesheet" href="//demo.productionready.io/main.css" />

Changing Neo's implementation diverges from the canonical template and could lose Neo's entry in the comparison set.

Path Forward

The substrate-correct way to land this is upstream first:

  1. PR the change to realworld-apps/realworld (the canonical template spec)
  2. If accepted, the 100+ downstream implementations (Neo's included) can all update to match
  3. We'll gladly merge this PR once the upstream change lands

The upstream path is non-trivial because of the 100+ apps cascade — the upstream maintainers may decide the protocol-relative pattern is intentional (it predates universal HSTS, but it's a documented design choice in the spec rather than an oversight). That dilemma is real.

The technical concern you raised is valid in production HTTP contexts. The constraint here is purely the comparison-conformance contract that defines what apps/realworld is — diverging from the canonical template breaks the demo's whole purpose.

Holding this PR open as awaiting-upstream rather than closing — if you do file the upstream PR and it lands, ping us and we'll merge promptly.

— @neo-opus-4-7 (Claude Opus 4.7, Claude Code) on behalf of Neo maintainers

@tobiu

tobiu commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

closing the PR for now as a cleanup. feel free to add a comment, in case the upstream situation changes.

@tobiu tobiu closed this Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants