Skip to content

Commit 709a456

Browse files
committed
test: pin the handling of configs from outside
Two doors take a config nobody here wrote: the JSON import, and whatever is in localStorage from a previous visit or another script on the origin. Both hold up. The sanitiser walks the keys of the defaults rather than the keys of the input, so an unknown key has nothing to be copied into and __proto__ never reaches a write. That is worth pinning because the obvious refactor is to iterate the input instead, which reads like a tidy-up: making that change, or assigning the input over the result, fails these.
1 parent 438dfb2 commit 709a456

1 file changed

Lines changed: 127 additions & 0 deletions

File tree

src/untrusted-input.spec.ts

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
import { TestBed } from "@angular/core/testing";
2+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
3+
import { sanitizeConfig } from "./models/config-serialization";
4+
import { DEFAULT_KITTY_CONFIG } from "./models/kitty-defaults";
5+
import { ConfigStoreService } from "./services/config-store.service";
6+
7+
/**
8+
* Two doors take a config nobody here wrote: the JSON import, and whatever is
9+
* sitting in localStorage from a previous visit or another script on the
10+
* origin. Both are read straight into the object the whole app renders from.
11+
*
12+
* The sanitiser walks the keys of the defaults rather than the keys of the
13+
* input, so an unknown key has nothing to be copied into. These pin that down,
14+
* because the obvious refactor is to iterate the input instead and it would
15+
* look like a tidy-up.
16+
*/
17+
describe("configs from outside", () => {
18+
beforeEach(() => {
19+
localStorage.clear();
20+
TestBed.resetTestingModule();
21+
});
22+
afterEach(() => {
23+
localStorage.clear();
24+
// Proves a leak rather than carrying it into the next test.
25+
delete (Object.prototype as Record<string, unknown>)["polluted"];
26+
});
27+
28+
const POLLUTING = [
29+
'{"__proto__":{"polluted":"yes"}}',
30+
'{"constructor":{"prototype":{"polluted":"yes"}}}',
31+
'{"fonts":{"__proto__":{"polluted":"yes"}}}',
32+
'{"fonts":{"constructor":{"prototype":{"polluted":"yes"}}}}',
33+
'{"__proto__":{"font_size":99}}',
34+
];
35+
36+
it.each(POLLUTING)("does not let %s reach Object.prototype", (json) => {
37+
const store = TestBed.inject(ConfigStoreService);
38+
39+
store.importFromJSON(json);
40+
41+
expect(({} as Record<string, unknown>)["polluted"]).toBeUndefined();
42+
expect(({} as Record<string, unknown>)["font_size"]).toBeUndefined();
43+
expect(store.configState().fonts.font_size).toBe(
44+
DEFAULT_KITTY_CONFIG.fonts.font_size,
45+
);
46+
});
47+
48+
it("keeps nothing the config does not already have a place for", () => {
49+
const sanitized = sanitizeConfig({
50+
fonts: { font_size: 14, not_a_real_option: "kept?" },
51+
also_not_a_section: { anything: 1 },
52+
}) as unknown as Record<string, Record<string, unknown>>;
53+
54+
expect(sanitized["fonts"]?.["font_size"]).toBe(14);
55+
expect(sanitized["fonts"]?.["not_a_real_option"]).toBeUndefined();
56+
expect(sanitized["also_not_a_section"]).toBeUndefined();
57+
});
58+
59+
it.each([
60+
["a string where a section goes", '{"fonts":"nope"}'],
61+
["a list where a section goes", '{"fonts":[1,2,3]}'],
62+
["null everywhere", '{"fonts":null,"colors":null}'],
63+
["a section of the wrong shape", '{"colors":{"color0":{"nested":true}}}'],
64+
["shortcuts that are not shortcuts", '{"keyboard_shortcuts":[1,"two",null]}'],
65+
["a number where a string goes", '{"fonts":{"font_family":42}}'],
66+
["a string where a number goes", '{"fonts":{"font_size":"big"}}'],
67+
["a config that is a list", "[1,2,3]"],
68+
["a config that is a string", '"hello"'],
69+
["a config that is null", "null"],
70+
])("survives %s with the shape intact", (_label, json) => {
71+
const store = TestBed.inject(ConfigStoreService);
72+
73+
expect(() => store.importFromJSON(json)).not.toThrow();
74+
75+
const config = store.configState() as unknown as Record<string, unknown>;
76+
const defaults = DEFAULT_KITTY_CONFIG as unknown as Record<string, unknown>;
77+
const wrong: string[] = [];
78+
for (const [section, expected] of Object.entries(defaults)) {
79+
const actual = config[section];
80+
if (Array.isArray(expected)) {
81+
if (!Array.isArray(actual)) wrong.push(section);
82+
} else if (typeof expected === "object" && expected !== null) {
83+
if (typeof actual !== "object" || actual === null || Array.isArray(actual)) {
84+
wrong.push(section);
85+
}
86+
} else if (typeof actual !== typeof expected) {
87+
wrong.push(section);
88+
}
89+
}
90+
91+
expect(wrong).toEqual([]);
92+
});
93+
94+
it("refuses a file that is not JSON without touching what is loaded", () => {
95+
const store = TestBed.inject(ConfigStoreService);
96+
store.updateField("fonts", "font_size", 17);
97+
98+
expect(store.importFromJSON("this is not json")).toBe(false);
99+
expect(store.configState().fonts.font_size).toBe(17);
100+
});
101+
102+
it("starts clean when localStorage holds something hostile", () => {
103+
// Any script on the origin can write here, and so can a person with the
104+
// console open.
105+
localStorage.setItem(
106+
"confitty-config",
107+
JSON.stringify({
108+
version: 1,
109+
config: { __proto__: { polluted: "yes" }, fonts: { font_size: 13 } },
110+
}),
111+
);
112+
113+
const store = TestBed.inject(ConfigStoreService);
114+
115+
expect(({} as Record<string, unknown>)["polluted"]).toBeUndefined();
116+
expect(typeof store.configState().fonts.font_size).toBe("number");
117+
});
118+
119+
it("starts clean when localStorage holds nonsense", () => {
120+
localStorage.setItem("confitty-config", "{{{ not json");
121+
122+
expect(() => TestBed.inject(ConfigStoreService)).not.toThrow();
123+
expect(TestBed.inject(ConfigStoreService).configState().fonts).toEqual(
124+
DEFAULT_KITTY_CONFIG.fonts,
125+
);
126+
});
127+
});

0 commit comments

Comments
 (0)