Skip to content

Commit c1f8858

Browse files
ZviBaratzSolomon
andauthored
fix(ego-lint): don't flag gi:// strings passed to importInShellOnly/importInPrefsOnly (#162)
## Problem Field test against Burn-My-Windows v48 (2.4M EGO downloads) produced **14 false positive FAILs** from `imports/shared-module-shell` and `imports/no-gtk-in-extension`. Closes #159. The extension uses a conditional import utility pattern in `src/utils.js`: ```js export async function importInShellOnly(module) { if (typeof global !== 'undefined') { return (await import(module)).default; } return null; } ``` Effect modules reference Shell libraries via this guard: ```js const Clutter = await utils.importInShellOnly('gi://Clutter'); ``` The previous grep matched `gi://Clutter` anywhere in the file, including as a quoted string argument. These are not imports — they're string arguments to guard functions that guarantee the import never executes in the wrong process context. ## Fix Filter grep output to only flag lines that contain an actual import statement (`from '...'` or `import('...')`). String references like `importInShellOnly('gi://Clutter')` don't contain either pattern and are no longer flagged. Applied to both: - `imports/shared-module-shell` (Shell imports in prefs-reachable modules) - `imports/no-gtk-in-extension` (GTK imports in extension-reachable modules) ## Test Coverage Added `import-guarded@test` fixture with the exact `importInShellOnly`/`importInPrefsOnly` pattern from Burn-My-Windows. Two new assertions verify no false positives are produced. ## Impact - Burn-My-Windows v48: 14 FP FAILs → 0 (only the real R-SEC-22 dconf FAIL remains) - Existing `shared-import-violation@test` still fires — real violations are unaffected 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Solomon <sol@nanoclaw.dev>
1 parent 0d79399 commit c1f8858

File tree

7 files changed

+70
-3
lines changed

7 files changed

+70
-3
lines changed

skills/ego-lint/scripts/check-imports.sh

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,9 @@ for f in "${runtime_files[@]}"; do
8181
rel_path="${f#"$EXT_DIR/"}"
8282
echo "FAIL|imports/no-gtk-in-extension|$rel_path: $match"
8383
violations=$((violations + 1))
84-
done < <(grep -nE "$gtk_pattern" "$f" 2>/dev/null | grep -vE "gi://GdkPixbuf" || true)
84+
done < <(grep -nE "$gtk_pattern" "$f" 2>/dev/null \
85+
| grep -vE "gi://GdkPixbuf" \
86+
| grep -E "(from\s+['\"]|import\s*\()" || true)
8587
done
8688

8789
# ---------------------------------------------------------------------------
@@ -146,14 +148,17 @@ if [[ -f "$EXT_DIR/prefs.js" ]]; then
146148
done < <(get_local_imports "$current")
147149
done
148150

149-
# Check prefs-reachable modules (excluding prefs.js itself) for Shell imports
151+
# Check prefs-reachable modules (excluding prefs.js itself) for Shell imports.
152+
# Only flag actual import statements (from '...' or import('...')) — not gi:// strings
153+
# passed as arguments to guard functions like importInShellOnly('gi://Clutter').
150154
for f in "${!prefs_visited[@]}"; do
151155
[[ "$f" == "$EXT_DIR/prefs.js" ]] && continue
152156
rel_path="${f#"$EXT_DIR/"}"
153157
while IFS= read -r match; do
154158
echo "FAIL|imports/shared-module-shell|$rel_path: Shell runtime import in module reachable from prefs.js: $match"
155159
violations=$((violations + 1))
156-
done < <(grep -nE "$shell_pattern" "$f" 2>/dev/null || true)
160+
done < <(grep -nE "$shell_pattern" "$f" 2>/dev/null \
161+
| grep -E "(from\s+['\"]|import\s*\()" || true)
157162
done
158163
fi
159164

tests/assertions/import-segregation.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,10 @@ run_lint "shared-import-violation@test"
77
assert_exit_code "exits with 1 (has failures)" 1
88
assert_output_contains "fails on shared module Shell import" "\[FAIL\].*imports/shared-module-shell"
99
echo ""
10+
11+
# --- import-guarded: importInShellOnly/importInPrefsOnly guard pattern ---
12+
echo "=== import-guarded ==="
13+
run_lint "import-guarded@test"
14+
assert_output_not_contains "no false positive for importInShellOnly string arg" "imports/shared-module-shell"
15+
assert_output_not_contains "no false positive for importInPrefsOnly Gtk string arg" "imports/no-gtk-in-extension"
16+
echo ""
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
GPL-2.0-or-later
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import {Extension} from 'resource:///org/gnome/shell/extensions/extension.js';
2+
import * as utils from './utils.js';
3+
4+
export default class GuardedExtension extends Extension {
5+
async enable() {
6+
// Shell-only imports via guard — safe, prefs process gets null
7+
const Clutter = await utils.importInShellOnly('gi://Clutter');
8+
const St = await utils.importInShellOnly('gi://St');
9+
if (Clutter) {
10+
this._actor = new Clutter.Actor();
11+
}
12+
}
13+
14+
disable() {
15+
this._actor = null;
16+
}
17+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "Import Guarded Test",
3+
"description": "Extension using importInShellOnly/importInPrefsOnly guard pattern",
4+
"uuid": "import-guarded@test",
5+
"version": 1,
6+
"shell-version": ["45", "46", "47", "48", "49", "50"],
7+
"url": "https://example.com"
8+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import {ExtensionPreferences} from 'resource:///org/gnome/Shell/Extensions/js/extensions/prefs.js';
2+
import * as utils from './utils.js';
3+
4+
export default class GuardedPrefs extends ExtensionPreferences {
5+
async fillPreferencesWindow(window) {
6+
// Prefs-only import via guard — safe, Shell process gets null
7+
const Gtk = await utils.importInPrefsOnly('gi://Gtk');
8+
if (Gtk) {
9+
const box = new Gtk.Box();
10+
window.add(box);
11+
}
12+
}
13+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Conditional import utilities — safe guard pattern used by Burn-My-Windows and similar extensions.
2+
// These functions ensure Shell-only modules never load in the prefs process (and vice versa).
3+
4+
export async function importInShellOnly(module) {
5+
if (typeof global !== 'undefined') {
6+
return (await import(module)).default;
7+
}
8+
return null;
9+
}
10+
11+
export async function importInPrefsOnly(module) {
12+
if (typeof global === 'undefined') {
13+
return (await import(module)).default;
14+
}
15+
return null;
16+
}

0 commit comments

Comments
 (0)