ReviewOS

also looking at this

stacks/lastsoulultra

LSU's own coaching surface, and what auditing the site turned up

#12
Merged glennmichael123 wants to merge claude/last-soul-ultra-web-6e0718 into main
86 files +3943 -1190

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

scripts/audit-html.tsmodified+49-0
Changes to scripts/audit-html.ts
@@ -117,6 +117,55 @@ export function auditPage(page: string, raw: string, served: Set<string>): Findi
117117 if (!/<html\b[^>]*\slang="[^"]+"/i.test(raw))
118118 findings.push({ page, rule: 'no-lang', detail: '' })
119119
120 // 7. A new tab the opener cannot be steered from. Without rel=noopener the
121 // opened page gets window.opener and can navigate this one elsewhere.
122 for (const [tag] of body.matchAll(/<a\b[^>]*>/gi)) {
123 if (attr(tag, 'target') !== '_blank') continue
124 if ((attr(tag, 'rel') ?? '').split(/\s+/).includes('noopener')) continue
125 findings.push({ page, rule: 'blank-no-noopener', detail: attr(tag, 'href') ?? '' })
126 }
127
128 // 8. An outbound link over plain HTTP, which a network between the reader
129 // and the host can read and rewrite. Loopback is the local preview.
130 for (const [, href] of body.matchAll(/<a\b[^>]*\shref="(http:\/\/[^"]+)"/gi)) {
131 const host = new URL(href!).hostname
132 if (host === 'localhost' || host === '127.0.0.1') continue
133 findings.push({ page, rule: 'insecure-link', detail: href! })
134 }
135
136 // 9. A control a screen reader can announce: an explicit label, an ARIA
137 // name, or the <label> it sits inside.
138 for (const match of body.matchAll(/<(input|select|textarea)\b[^>]*>/gi)) {
139 const tag = match[0]
140 const type = attr(tag, 'type') ?? ''
141 if (['hidden', 'submit', 'button', 'reset', 'image'].includes(type)) continue
142 if (attr(tag, 'aria-label') || attr(tag, 'aria-labelledby') || attr(tag, 'title')) continue
143 const id = attr(tag, 'id')
144 if (id && new RegExp(`<label\\b[^>]*\\sfor="${id}"`, 'i').test(body)) continue
145 // Wrapped in a label: the nearest <label> before it is still open.
146 const before = body.slice(0, match.index)
147 const opened = before.lastIndexOf('<label')
148 if (opened !== -1 && !before.slice(opened).includes('</label>')) continue
149 findings.push({ page, rule: 'control-no-label', detail: `${match[1]} ${attr(tag, 'name') ?? type}` })
150 }
151
152 // 10. A button with a name. Text bound at runtime counts: the markup is
153 // empty on purpose and the binding fills it.
154 for (const [, tag, inner] of body.matchAll(/(<button\b[^>]*>)([\s\S]*?)<\/button>/gi)) {
155 if (inner!.replace(/<[^>]+>/g, '').replace(/&[a-z]+;/gi, ' ').trim()) continue
156 if (attr(tag!, 'aria-label') || attr(tag!, 'title')) continue
157 if (/\s(?::text|x-text)=/i.test(tag!)) continue
158 if (/<img\b[^>]*\salt="[^"]+"/i.test(inner!)) continue
159 findings.push({ page, rule: 'button-no-name', detail: tag!.slice(0, 60) })
160 }
161
162 // 11. What the page says it is, for a search engine and a share sheet.
163 if (!/<meta name="description" content="[^"]{20,}"/i.test(raw))
164 findings.push({ page, rule: 'no-description', detail: '' })
165 const canonical = /<link rel="canonical" href="([^"]+)"/i.exec(raw)?.[1]
166 if (!canonical || !canonical.startsWith('http'))
167 findings.push({ page, rule: 'no-canonical', detail: canonical ?? '' })
168
120169 return findings
121170}
122171
tests/html-audit.test.tsmodified+80-0
Changes to tests/html-audit.test.ts
@@ -52,3 +52,83 @@ describe('every page the site serves', () => {
5252 expect(views.map(view => view.url)).toContain('/coaching/feedback')
5353 })
5454})
55
56/**
57 * A rule whose pattern stopped matching reports nothing, which looks exactly
58 * like a clean site. Each one is given a page it must complain about and a
59 * page it must not.
60 */
61describe('each rule', () => {
62 const served = new Set(['/', '/format'])
63
64 function rules(html: string): string[] {
65 const page = `<!DOCTYPE html><html lang="en"><head><title>t</title><meta name="description" content="A description long enough to count as one."><link rel="canonical" href="https://example.test/"></head><body><h1>One</h1>${html}</body></html>`
66 return auditPage('/t', page, served).map(finding => finding.rule)
67 }
68
69 it('catches a link to a page that is not served', () => {
70 expect(rules('<a href="/gone">gone</a>')).toContain('dead-link')
71 expect(rules('<a href="/format">here</a>')).not.toContain('dead-link')
72 expect(rules('<a href="/assets/x.css">asset</a>')).not.toContain('dead-link')
73 })
74
75 it('catches an image with no alt', () => {
76 expect(rules('<img src="/a.png">')).toContain('img-no-alt')
77 expect(rules('<img src="/a.png" alt="">')).not.toContain('img-no-alt')
78 })
79
80 it('catches two elements sharing an id', () => {
81 expect(rules('<p id="x"></p><p id="x"></p>')).toContain('duplicate-id')
82 expect(rules('<p id="x"></p><p id="y"></p>')).not.toContain('duplicate-id')
83 })
84
85 it('catches a skipped heading level and a second h1', () => {
86 expect(rules('<h3>skip</h3>')).toContain('heading-skip')
87 expect(rules('<h2>fine</h2><h3>fine</h3>')).not.toContain('heading-skip')
88 expect(rules('<h1>two</h1>')).toContain('h1-count')
89 })
90
91 it('catches a link with nothing to announce', () => {
92 expect(rules('<a href="/format"><span class="icon"></span></a>')).toContain('link-no-name')
93 expect(rules('<a href="/format" aria-label="Home"><span></span></a>')).not.toContain('link-no-name')
94 expect(rules('<a href="/format"><img src="/a.png" alt="Home"></a>')).not.toContain('link-no-name')
95 })
96
97 it('catches a new tab the opener can be steered from', () => {
98 expect(rules('<a href="/format" target="_blank">x</a>')).toContain('blank-no-noopener')
99 expect(rules('<a href="/format" target="_blank" rel="noopener">x</a>')).not.toContain('blank-no-noopener')
100 })
101
102 it('catches an outbound link over plain HTTP, but not the local preview', () => {
103 expect(rules('<a href="http://hq.training/x">x</a>')).toContain('insecure-link')
104 expect(rules('<a href="http://localhost:3940/x">x</a>')).not.toContain('insecure-link')
105 expect(rules('<a href="https://hq.training/x">x</a>')).not.toContain('insecure-link')
106 })
107
108 it('catches a control with no label, in any of the four ways of giving it one', () => {
109 expect(rules('<input name="email">')).toContain('control-no-label')
110 expect(rules('<label>Email<input name="email"></label>')).not.toContain('control-no-label')
111 expect(rules('<label for="e">Email</label><input id="e" name="email">')).not.toContain('control-no-label')
112 expect(rules('<input name="email" aria-label="Email">')).not.toContain('control-no-label')
113 expect(rules('<input type="hidden" name="token">')).not.toContain('control-no-label')
114 })
115
116 it('catches a button with no name, and allows one whose text is bound', () => {
117 expect(rules('<button type="submit"></button>')).toContain('button-no-name')
118 expect(rules('<button :text="label"></button>')).not.toContain('button-no-name')
119 expect(rules('<button aria-label="Close">×</button>')).not.toContain('button-no-name')
120 })
121
122 it('catches a page that does not say what it is', () => {
123 const bare = auditPage('/t', '<!DOCTYPE html><html lang="en"><body><h1>x</h1></body></html>', served).map(f => f.rule)
124 expect(bare).toContain('no-description')
125 expect(bare).toContain('no-canonical')
126 expect(rules('')).not.toContain('no-description')
127 expect(rules('')).not.toContain('no-canonical')
128 })
129
130 it('catches a page with no language', () => {
131 expect(auditPage('/t', '<html><body><h1>x</h1></body></html>', served).map(f => f.rule)).toContain('no-lang')
132 expect(rules('')).not.toContain('no-lang')
133 })
134})