Close two holes in the privacy ship-gate !22

merged merged by cmc on 2026-09-04 16:50 UTC · krz/ambient-companions:fix/privacy-scan-holes into main

1 file changed, +95 −13

crates/signal-schema/tests/privacy_invariant.rs +95 −13
@@ -80,23 +80,90 @@ fn wire_format_has_no_content_field() {
8080/// The static gate: the tree must contain none of the APIs a
8181/// real keylogger would use, nor any shell hook that touches the line buffer.
8282/// Any hit fails the build. The banned list itself lives here and is reviewed.
83// APIs for input taps / global key monitoring, and shell line-buffer refs.
84//
85// Prefixes where a prefix is safe: `CGEvent` covers `CGEventTap` and the
86// rest of the family, `IOHID` covers `IOHIDManager`, `IOHIDQueue`,
87// `IOHIDDevice` and `IOHIDElement`.
88//
89// Bare `NSEvent` is deliberately absent. The keylogger-shaped API is the
90// monitor, not the class, and a menu-bar face will need `NSEvent` to draw
91// a UI. Banning the class would cost future work and buy no safety.
92//
93// The zle parameters are listed in both `$X` and `${X}` form: `${BUFFER}`
94// is valid zsh and does not contain the substring `$BUFFER`, so the sigil
95// form alone was bypassable by two characters.
96const BANNED: &[&str] = &[
97 "CGEvent",
98 "IOHID",
99 "kAXTrusted",
100 "AXObserver",
101 "AXUIElement",
102 "addGlobalMonitorForEvents",
103 "addLocalMonitorForEvents",
104 "$BUFFER",
105 "${BUFFER}",
106 "$LBUFFER",
107 "${LBUFFER}",
108 "$RBUFFER",
109 "${RBUFFER}",
110];
111
112/// The banned tokens appearing in `text`.
113fn violations(text: &str) -> Vec<&'static str> {
114 BANNED.iter().copied().filter(|b| text.contains(b)).collect()
115}
116
117/// The scan passing proves the tree is clean; it proves nothing about whether
118/// the gate would catch a violation. These pin what it catches, and — just as
119/// importantly — what it deliberately does not.
120#[test]
121fn the_gate_catches_real_violations_and_leaves_legitimate_code_alone() {
122 // Each of these is how the corresponding API actually gets written.
123 for (sample, why) in [
124 ("local saved=${BUFFER}", "braced zle buffer — the form that used to slip through"),
125 ("print -r -- $RBUFFER", "the right half of the line buffer"),
126 ("local x=${LBUFFER}", "braced left buffer"),
127 ("let tap = CGEventTapCreate(.cgSessionEventTap, ...)", "event tap"),
128 ("let q = IOHIDQueueCreate(kCFAllocatorDefault, dev, 8, 0)", "HID queue"),
129 ("IOHIDManagerRegisterInputValueCallback(mgr, cb, nil)", "HID manager"),
130 ("NSEvent.addLocalMonitorForEvents(matching: .keyDown) { $0 }", "local key monitor"),
131 ("NSEvent.addGlobalMonitorForEvents(matching: .keyDown) { _ in }", "global key monitor"),
132 ("AXObserverCreate(pid, callback, &observer)", "accessibility observation"),
133 ("AXUIElementCopyAttributeValue(el, kAXValueAttribute, &v)", "accessibility read"),
134 ] {
135 assert!(
136 !violations(sample).is_empty(),
137 "the gate would not catch {why}: {sample:?}"
138 );
139 }
140
141 // False positives cost future work. A menu-bar face needs NSEvent to draw
142 // a UI, our own hook rebinds self-insert, and prose naming the parameters
143 // is how the privacy contract is documented.
144 for (sample, why) in [
145 ("let p = NSEvent.mouseLocation", "NSEvent for UI, not monitoring"),
146 ("zle -N self-insert _signald_self_insert", "our own keystroke counter"),
147 ("# never reference the BUFFER/LBUFFER/RBUFFER zle parameters", "prose in the contract"),
148 ("ProcessInfo.processInfo.thermalState", "an ordinary aggregate read"),
149 ] {
150 assert!(
151 violations(sample).is_empty(),
152 "the gate false-positives on {why}: {sample:?} -> {:?}",
153 violations(sample)
154 );
155 }
156}
157
83158#[test]
84159fn forbidden_symbol_scan() {
85 // APIs for input taps / global key monitoring, and shell line-buffer refs.
86 const BANNED: &[&str] = &[
87 "CGEventTap",
88 "IOHIDManager",
89 "kAXTrusted",
90 "addGlobalMonitorForEvents",
91 "$BUFFER",
92 "$LBUFFER",
93 ];
94160 const CODE_EXTS: &[&str] = &["rs", "zsh", "sh", "swift", "m", "c", "h"];
95161
96162 let this_file = PathBuf::from(file!());
97163 let this_name = this_file.file_name().unwrap();
98164
99165 let mut offenders = Vec::new();
166 let mut scanned: Vec<String> = Vec::new();
100167 for path in walk(&repo_root()) {
101168 // Skip build artifacts and this test (which names the banned tokens).
102169 // `target` is Rust's; `.build` is SwiftPM's (macos-collector/.build).
@@ -120,10 +187,9 @@ fn forbidden_symbol_scan() {
120187 let Ok(text) = std::fs::read_to_string(&path) else {
121188 continue;
122189 };
123 for banned in BANNED {
124 if text.contains(banned) {
125 offenders.push(format!("{}: {banned}", path.display()));
126 }
190 scanned.push(path.display().to_string());
191 for banned in violations(&text) {
192 offenders.push(format!("{}: {banned}", path.display()));
127193 }
128194 }
129195
@@ -132,6 +198,22 @@ fn forbidden_symbol_scan() {
132198 "forbidden keylogger symbol(s) found:\n{}",
133199 offenders.join("\n")
134200 );
201
202 // A gate that scans nothing reports success while checking nothing, which
203 // is worse than having no gate. Assert it actually read the files that
204 // matter: the shell hook is where a line-buffer reference would appear,
205 // and the Swift collector is where an input-tap API would.
206 assert!(
207 scanned.len() >= 10,
208 "scan read only {} files; the walk is broken",
209 scanned.len()
210 );
211 for required in ["signald-hooks.zsh", "Hardware.swift"] {
212 assert!(
213 scanned.iter().any(|p| p.ends_with(required)),
214 "scan never reached {required}; it is not covering the tree.\nscanned: {scanned:#?}"
215 );
216 }
135217}
136218
137219/// The differential secret-typing acceptance test — the ship gate for the