Get this on every PR automatically. Webhook fires, AI reviews, you approve.
Install on langchain-ai/openwiki →
// reviewed by pulllight

PullLight found 6 issues

langchain-ai/openwiki #165 — feat: enforce .openwikiignore rules by @n33levo Jul 18, 2026
2 high 4 medium
🔍
// convert to your repo
PullLight caught 6 2 high, 4 medium on this PR.
Install on langchain-ai/openwiki to get this on every PR automatically — 60 seconds.
Findings
6 issues
medium logic
`matchesRule` returns `true` for a `directoryOnly` rule when `filePath.includes("/")` — meaning any file nested under a directory (e.g. `src/foo.txt`) will satisfy the directory-only check even if the rule was meant to match only directories. A `directoryOnly` rule should only match when `isDirectory` is `true`; the `filePath.includes("/")` fallback incorrectly widens the match to regular files inside subdirectories.
// src/agent/openwiki-ignore.ts:155
medium logic
`extractGitPaths` tries both `shortStatusMatch` and `nameStatusMatch` but uses `??` so `nameStatusMatch` is only tried when `shortStatusMatch` is null. However, `git log --name-status` emits lines like `M\tpath/to/file` which start with a single status letter — these will fail the short-status regex (which requires two characters before the space) and fall through to `nameStatusMatch`. But commit-message lines (e.g. `abc1234 Fix bug`) also won't match either regex and will correctly return `[]`. The real risk is rename lines from `--name-status`: `R100\told/path\tnew/path` — `splitGitPaths` splits on `\t` and returns both paths, which is correct. However, the short-status regex `^(?:[ MARCUD?!]{2})\s+(.+)$` will also match two-char prefixes like `??` or `!!` (untracked/ignored markers from `git status --short`) and capture the rest including a potential second path separated by ` -> ` for renames in `git status --short` output (`R old -> new`). `splitGitPaths` handles ` -> ` but only after the tab check, so a status-short rename line `R old -> new` would be split correctly. This appears fine, but the regex `[ MARCUD?!]{2}` does not include `R` in the second position for staged renames — `RM`, `RD` etc. are valid two-char codes. This is a minor logic gap but unlikely to cause a security issue.
// src/agent/utils.ts:530
high logic
The `allowedIgnoredShellCommands` allowlist uses `allowedCommand.test(trimmedCommand)` with stateful `RegExp` objects. None of the three regexes use the `g` or `y` flag, so `lastIndex` is not an issue here — but the `rm -f` pattern `/^rm\s+-f\s+(?:\.\/)?openwiki\/_plan\.md$/u` allows deletion of the plan file even while ignore rules are active. More critically, the allowlist is the *only* gate when `ignoreRules.isActive` is true: any command that matches one of these patterns bypasses the shell restriction entirely. The `git rev-parse HEAD` pattern `/^git\s+(?:--no-pager\s+)?rev-parse\s+HEAD$/u` is fine, but the `rm` pattern could be abused if the model is prompted to issue `rm -f openwiki/_plan.md` to clear the plan and then chain additional shell commands — though chaining is blocked by the `$` anchor. The patterns look safe as written, but the `rm` allowance is a surprising exception worth documenting explicitly.
// src/agent/docs-only-backend.ts:218
medium logic
`lineReferencesIgnoredPath` is used to filter `git status --short` output in `getUpdateNoopStatus`. If a line does not match either the short-status or name-status regex in `extractGitPaths`, it returns `[]` and the line is kept. Untracked file lines from `git status --short` start with `??` — `?` is included in the short-status character class so these will be parsed. However, section headers or other non-path lines that happen to start with two characters from `[ MARCUD?!]` followed by a space could be misidentified. More concretely: if `extractGitPaths` returns `[]` for a line that actually references an ignored path (because the regex didn't match), that line will *not* be filtered, potentially causing `getUpdateNoopStatus` to report `shouldSkip: false` when it should be `true`. This is a correctness gap rather than a security issue.
// src/agent/utils.ts:113
medium logic
In `createPatternMatcher`, when a pattern contains a slash (e.g. `logs/keep.log`) it is treated as anchored with `^${source}(?:/.*)?$`. This means the negation pattern `!logs/keep.log` will only un-ignore `logs/keep.log` when the normalized path is exactly `logs/keep.log` or starts with `logs/keep.log/`. But `normalizeIgnorePath` strips leading slashes, so a path passed in as `/logs/keep.log` becomes `logs/keep.log` — that part is fine. However, if the backend receives a path like `./logs/keep.log`, `normalizeIgnorePath` strips the leading `./` via the `replace(/^\/+/u, "")` call — but `./` is not a leading slash, it is a dot-slash. The `normalizeIgnorePath` function only strips leading *slashes* (`/+`), not `./`. So `./logs/keep.log` would normalize to `./logs/keep.log` (the backslash replacement runs, but there are no backslashes; the leading-slash strip does nothing since it starts with `.`). This means `ignores("./logs/keep.log")` would not match the negation rule and the file would remain ignored.
// src/agent/openwiki-ignore.ts:128
high performance.n_plus_one
**N+1 query detected**: A database call (`const blocked = await backend.execute("cat secrets/token.txt");`) runs inside a loop (`expect(glob.files?.map((file) => file.path).join("\n")).not.toContain(`), firing one query per iteration. Under load this will degrade response times and hammer your database connection pool. **Fix**: Move the database call outside the loop. Collect all required IDs/keys first, fetch them in a single batched query, then look up results from a Map inside the loop.
// test/openwiki-ignore.test.ts:97
model: claude-sonnet-4-5   input: 16360 tokens   output: 1877 tokens