1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
//! Guard: repository strings reaching an HTML sink in the SPA are escaped.
//!
//! The dashboard embeds its data as JSON and every widget builds markup by
//! string concatenation, so any repository-derived string — a path, a module
//! name, an author — is one missing `escapeHtml` away from executing in the
//! viewer's browser. Path names may legally contain `<` and `>` on Linux and
//! macOS, git tracks them verbatim, and the emitter's only JSON defence is
//! `"</"` → `"<\\/"`, which prevents `</script>` breakout and nothing else:
//! after `JSON.parse` the string carries its metacharacters intact, and the
//! next `innerHTML` concatenation is a fresh injection point that the
//! transport-level fix has no jurisdiction over. Escaping has to happen at
//! the sink, which is why the house convention is `escapeHtml` there.
//!
//! The class recurs. One widget built an `onclick` by concatenating row data
//! into an attribute; three cycles later a different widget concatenated
//! module paths into two chart tooltips — same defect, new file, because
//! nothing enforced the convention. This guard is the enforcement.
//!
//! What it checks: in any statement that also builds markup, an accessor
//! naming a repository-derived string must sit inside `escapeHtml(...)`.
//! Numeric fields are not listed — they cannot carry markup — and the
//! accessor list is derived from the JSON payload's string fields rather
//! than from an exemption list, so it does not rot as widgets change.
//!
//! What it does not check: markup assembled across statement boundaries, or
//! a field added to the payload without being added below. It is a
//! convention guard, not a taint tracker; the statement of its limits is
//! part of the guard.
/// Every widget source, in the order `output::spa` concatenates them.
const WIDGET_SOURCES: &[(&str, &str)] = &[
(
"00_setup_boot.js",
include_str!("../src/output/spa/js/00_setup_boot.js"),
),
(
"10_helpers.js",
include_str!("../src/output/spa/js/10_helpers.js"),
),
(
"12_drawer.js",
include_str!("../src/output/spa/js/12_drawer.js"),
),
(
"14_widgets_summary.js",
include_str!("../src/output/spa/js/14_widgets_summary.js"),
),
(
"16_widgets_bars.js",
include_str!("../src/output/spa/js/16_widgets_bars.js"),
),
(
"20_hotspots.js",
include_str!("../src/output/spa/js/20_hotspots.js"),
),
(
"30_coupling_trends.js",
include_str!("../src/output/spa/js/30_coupling_trends.js"),
),
(
"40_architecture.js",
include_str!("../src/output/spa/js/40_architecture.js"),
),
(
"50_calendar_xray.js",
include_str!("../src/output/spa/js/50_calendar_xray.js"),
),
(
"90_toggles_utils.js",
include_str!("../src/output/spa/js/90_toggles_utils.js"),
),
];
/// Accessors whose value is a repository-derived string. Taken from the
/// string fields of the SPA JSON payload plus the two chart-library
/// carriers (`p.name`, `order[...]`) that receive them.
const RAW_STRING_ACCESSORS: &[&str] = &[
".path",
".entity",
".entity_a",
".entity_b",
".author",
".canonical_author",
".source",
".target",
".module",
".tag",
".name",
"order[",
];
/// Markers that a statement is building markup rather than plain text.
/// Written without the entity-terminating `;` on purpose: statements are
/// split on `;`, which would otherwise cut every entity in half and make
/// the markers unmatchable.
const HTML_MARKERS: &[&str] = &[
"innerHTML",
"<br",
"<div",
"<span",
"<strong",
"<p ",
"&rarr",
"&harr",
"·",
"title=\"",
];
/// Statement-ish slices. Markup here is built by concatenation terminated
/// by `;`, so splitting there keeps a multi-line `return` whole while
/// separating unrelated code.
///
/// HTML entities end in `;` too. Splitting on those would cut a statement
/// at `→` — severing it from the very marker that identifies it as
/// markup, and hiding every accessor after the entity. So a `;` that closes
/// an entity is not a statement boundary.
fn statements(src: &str) -> Vec<String> {
let bytes = src.as_bytes();
let mut out = Vec::new();
let mut start = 0;
for (i, b) in bytes.iter().enumerate() {
if *b != b';' {
continue;
}
let name_start = src[..i]
.rfind(|c: char| !c.is_ascii_alphanumeric())
.map_or(0, |j| j + 1);
let closes_entity = name_start > 0 && name_start < i && bytes[name_start - 1] == b'&';
if !closes_entity {
out.push(src[start..i].to_owned());
start = i + 1;
}
}
out.push(src[start..].to_owned());
out
}
/// What precedes the member expression containing `pos`, with the
/// expression's own identifier chain walked off.
fn preceding(stmt: &str, pos: usize) -> &str {
let head = stmt[..pos]
.rfind(|c: char| !(c.is_ascii_alphanumeric() || c == '_' || c == '$' || c == '.'))
.map_or(0, |i| i + 1);
stmt[..head].trim_end()
}
/// Whether the expression at `pos` is already wrapped in `escapeHtml(...)`.
fn is_escaped(stmt: &str, pos: usize) -> bool {
preceding(stmt, pos).ends_with("escapeHtml(")
}
/// Whether the expression at `pos` is a subscript — `moduleRole[p.name]`
/// looks up a role by path, so the path is a key, not rendered output.
/// Escaping it would break the lookup rather than secure it.
fn is_lookup_key(stmt: &str, pos: usize) -> bool {
preceding(stmt, pos).ends_with('[')
}
/// Unescaped raw-string accessors inside markup-building statements.
fn unescaped_sinks(src: &str) -> Vec<String> {
let mut out = Vec::new();
for stmt in statements(src) {
if !HTML_MARKERS.iter().any(|m| stmt.contains(m)) {
continue;
}
for accessor in RAW_STRING_ACCESSORS {
let mut from = 0;
while let Some(rel) = stmt[from..].find(accessor) {
let at = from + rel;
if !is_escaped(&stmt, at) && !is_lookup_key(&stmt, at) {
let line = stmt[..at].lines().count();
out.push(format!("{accessor} (statement line ~{line})"));
}
from = at + accessor.len();
}
}
}
out
}
#[test]
fn no_widget_concatenates_repository_strings_into_markup_unescaped() {
let mut violations = Vec::new();
for (name, src) in WIDGET_SOURCES {
for hit in unescaped_sinks(src) {
violations.push(format!(" {name}: {hit}"));
}
}
assert!(
violations.is_empty(),
"{} SPA sink(s) interpolate a repository-derived string into markup \
without `escapeHtml`:\n{}\n\n\
Repository paths may contain `<` and `>`; they reach the browser \
verbatim because the emitter escapes only `</` in the JSON payload. \
An unescaped concatenation into `innerHTML` — including the return \
value of an ECharts function formatter, which is inserted as markup \
rather than filtered like a `{{b}}` template — executes whatever the \
analysed repository put in that path.\n\n\
Wrap the value in `escapeHtml(...)`, the helper every other widget \
already uses.",
violations.len(),
violations.join("\n"),
);
}
#[test]
fn the_guard_catches_the_shape_it_exists_for() {
// The real check passes when the tree is clean, which is also what a
// broken matcher looks like. Pin it against the defect it was written
// for — the architecture tooltips, in their pre-fix form — and against
// the fixed form, so neither a vacuous pass nor a false positive can
// hide. An earlier draft of this matcher required a `+` before the
// accessor and allowed only one member segment; it reported zero
// violations on the vulnerable code below.
let vulnerable =
"return 'Imports: ' + p.data.source + ' → ' + p.data.target + ' (' + n + ')';";
assert_eq!(
unescaped_sinks(vulnerable).len(),
2,
"must flag both unescaped edge endpoints"
);
let vulnerable_leading = "return p.name + '<br/>role: ' + role;";
assert_eq!(
unescaped_sinks(vulnerable_leading).len(),
1,
"must flag an accessor that opens the expression, with no `+` before it"
);
let vulnerable_index = "return order[r] + ' → ' + order[c] + '<br/>' + v;";
assert_eq!(
unescaped_sinks(vulnerable_index).len(),
2,
"must flag indexed axis labels"
);
let fixed =
"return 'Imports: ' + escapeHtml(p.data.source) + ' → ' + escapeHtml(p.data.target);";
assert_eq!(
unescaped_sinks(fixed).len(),
0,
"must accept the escaped form"
);
// Markup-free statements are out of scope even when they carry paths,
// or every data-plumbing line in the file would be a violation.
let not_markup = "const rm = modulePath(rr.path, chosenDepth);";
assert_eq!(
unescaped_sinks(not_markup).len(),
0,
"a statement that builds no markup is not a sink"
);
// A path used as a subscript is a key, not rendered output. Escaping it
// would change what is looked up rather than secure anything.
let lookup = "return escapeHtml(p.name) + '<br/>role: ' + (moduleRole[p.name] || 'periphery');";
assert_eq!(
unescaped_sinks(lookup).len(),
0,
"an accessor inside a subscript is a lookup key, not a sink"
);
}