Skip to content

fix: address spec hygiene issues from security review #135

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

Jump to bottom
Open
Sven-NOM wants to merge 15 commits into
base: staging
Choose a base branch
from
Open

fix: address spec hygiene issues from security review #135

Sven-NOM wants to merge 15 commits into from

Conversation

@Sven-NOM
Copy link
Contributor

@Sven-NOM Sven-NOM commented Jan 16, 2026 •
edited
Loading

Summary

This PR addresses multiple issues identified during a security/quality review of the spec:

Changes

Risk reference fixes (#130, #134)

  • Fixed label/link mismatches (DOW16→DOW18, GIR2→GIR25, FIN7→FIN6, etc.)
  • Removed duplicate risk entries (GIR15 x5, GIR21 x2, SLS3 x2, FIN1 x2)
  • Added missing numbers to FIN labels
  • Removed obsolete SLS16 reference

DOM construction hygiene (#131)

  • Replaced string concatenation + innerHTML with createElement/setAttribute/textContent
  • This is localized code hygiene - inputs are trusted DOM content

Vendored respec (#132)

  • Downloaded respec-w3c v35.6.1 to vendor/
  • Removes external dependency on w3.org CDN

Automated validation (#133)

  • Added .github/workflows/validate-risk-refs.yml
  • Added scripts/validate-risk-refs.sh
  • Checks: label/link matches, no duplicates, all referenced risks exist

Test plan

  • Run ./scripts/validate-risk-refs.sh locally - passes
  • Verify spec renders correctly with vendored respec
  • Review risk table links work as expected
  • Check if github workflow works properly (checked after PR was submitted)

Closes #130, closes #131, closes #132, closes #133, closes #134

@Sven-NOM
fix: broken links from issue #134
715f32e
@Sven-NOM
fix: correct inconsistent risk references in spec. Fixes #130
055eed6
@Sven-NOM
feat: added an automated validation for risk ID consistency. Fixes #133
e8a1824
@Sven-NOM
fix: vendor respec locally for supply-chain security
12032e9 
Fixes #132

- Download respec-w3c v35.6.1 to vendor/
- Remove external script dependency on w3.org
@Sven-NOM
fix: replace innerHTML with safe DOM construction
5b1a859 
Fixes #131

- Use createElement/setAttribute/textContent instead of string concatenation
- Localized hygiene fix, not a security boundary (inputs are trusted DOM)
@Sven-NOM Sven-NOM requested a review from hexnickk4997 January 16, 2026 09:52
chaals
chaals approved these changes Jan 16, 2026
View reviewed changes
Copy link
Collaborator

@chaals chaals left a comment

Choose a reason for hiding this comment

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

All this looks good (NB I didn't verify that you downloaded respec correctly)

Sven-NOM and others added 7 commits February 4, 2026 09:50
@Sven-NOM @ampcode-com
docs: add commit signing guidelines for contributors
2c027d0 
Amp-Thread-ID: https://ampcode.com/threads/T-019c2432-3307-7101-b685-aab98de24ae9
Co-authored-by: Amp <[email protected]>
@Sven-NOM
removed feedback form to Duck for issue #137
7f57ea8
@Sven-NOM @ampcode-com
feat: vendor fixup.js for CSP compliance and supply chain security
08a6ef6 
Co-authored-by: Amp <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-019c2432-3307-7101-b685-aab98de24ae9
@Sven-NOM @ampcode-com
feat: vendor fixup.js locally, whitelist W3C stylesheets in CSP
a782d8a 
- Modified respec-w3c-35.6.1.js to load fixup.js from local vendor path
- Updated CSP to allow W3C stylesheets (safe, non-executable) while keeping all JS vendored
- Updated checksums and documentation for all modifications

Amp-Thread-ID: https://ampcode.com/threads/T-019c2432-3307-7101-b685-aab98de24ae9
Co-authored-by: Amp <[email protected]>
@Sven-NOM
feat: finalize supply chain security for vendored scripts (#132)
e4f89a7 
- Update CSP to whitelist W3C scripts (https://www.w3.org/scripts/)
- Simplify respec modifications to single string replacement (respec-highlight.js)
- Remove vendored fixup.js (loads from W3C CDN, CSP-whitelisted)
- Update CHECKSUMS.md with verification instructions

Security model:
- respec-w3c-35.6.1.js: vendored locally, 1 documented modification
- respec-highlight.js: vendored locally, loaded via Web Worker
- axe.min.js: vendored locally, checksum verified
- fixup.js: W3C CDN with CSP domain whitelist (SRI not possible due to CORS)
@Sven-NOM
fix: broken links and footing wording to address issue #138
3f85c0d
@Sven-NOM
chore: update checksums.md to accurately represent replacements made.…
71fc785 
… Issue #140
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chaals chaals chaals approved these changes

@hexnickk4997 hexnickk4997 Awaiting requested review from hexnickk4997

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Sven-NOM @chaals @hexnickk4997