-
Notifications
You must be signed in to change notification settings - Fork 2.2k
bin/lib/*.js Files Have Zero Test Coverage — 16 Source Files With No Security Regression Tests - IssueFinder - SN 22 #1446
Description
Problem Statement
All 16 source files under bin/lib/ (credentials.js, policies.js, onboard.js, runner.js, registry.js, runtime-recovery.js, preflight.js, nim.js, local-inference.js, inference-config.js, platform.js, chat-filter.js, resolve-openshell.js, onboard-session.js, version.js) have zero unit test coverage. Security-critical functions like loadPreset() path traversal guard, shellQuote() command injection prevention, credential file permissions, and UNSAFE_HOME_PATHS validation are not tested.
Impact
Security regressions can be introduced without automated detection. Known vulnerability patterns (C-2 Dockerfile injection, C-4 manifest traversal) have no regression guards in the plugin codebase.
Affected Area
- File(s): All files under bin/lib/
- Service(s): CLI security infrastructure
Related GitHub Issue Check
- Matching open issue: refactor(test): wire E2E security tests to real production code paths #1107 (related — wire E2E security tests to real code paths)
- Matching closed issue: Add Brev E2E tests for command injection and credential sanitization #1097 (closed — added E2E security tests)
- Duplicate status: Related but distinct
- Reasoning: refactor(test): wire E2E security tests to real production code paths #1107 is about wiring existing E2E tests to production code. This finding is about the complete absence of unit tests for 16 bin/lib/ source files, including security-critical modules like credentials.js, policies.js, runner.js, and onboard.js.
Reproduction Steps
- Run the test suite with coverage:
npx vitest run --coverage
- Check coverage output for
bin/lib/:cat coverage/coverage-summary.json | python3 -c " import json, sys d = json.load(sys.stdin) for k, v in d.items(): if 'bin/lib' in k: print(f'{k}: {v["lines"]["pct"]}% lines') "
- Observe:
bin/lib/has 0% coverage — no test files exist for onboard.js, credentials.js, etc.
Debug Output
# View current coverage configuration:
cat vitest.config.ts | grep -A 10 'coverage'
# Note: coverage.include only covers nemoclaw/src/**/*.ts — not bin/lib/
# Run tests and check what's covered:
npx vitest run --coverage 2>&1 | tail -20
# bin/lib/ files will not appear in coverage reportLogs
# vitest coverage output (current):
% Coverage report generated
----------|---------|----------|---------|---------|
File | % Stmts | % Branch | % Funcs | % Lines |
----------|---------|----------|---------|---------|
nemoclaw/ | 62.5 | 55.0 | 70.0 | 62.5 |
----------|---------|----------|---------|---------|
# ↑ bin/lib/ not listed — 0% coverage, not tracked
Proposed Design
Recommended Fix
Step 1 — Expand coverage config in vitest.config.ts to include bin/lib/:
coverage: {
provider: "v8",
include: ["nemoclaw/src/**/*.ts", "bin/lib/**/*.js"],
exclude: ["**/*.test.ts", "**/*.test.js"],
reporter: ["text", "json-summary"],
thresholds: {
"bin/lib/**/*.js": {
lines: 40, // Start at 40%, ratchet up over time
functions: 40,
branches: 30,
},
"nemoclaw/src/**/*.ts": {
lines: 60,
functions: 60,
branches: 50,
},
},
},Step 2 — Update ci/coverage-threshold.json to include bin/lib:
{
"nemoclaw/src": { "lines": 60, "functions": 60, "branches": 50 },
"bin/lib": { "lines": 40, "functions": 40, "branches": 30 }
}Step 3 — Add priority test file test/credentials.test.js (starter):
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import fs from "fs";
import path from "path";
// Test credential storage security properties
describe("credentials", () => {
const tmpDir = path.join("/tmp", `nemoclaw-test-${process.pid}`);
beforeEach(() => {
fs.mkdirSync(tmpDir, { recursive: true });
process.env.HOME = tmpDir;
});
afterEach(() => {
fs.rmSync(tmpDir, { recursive: true, force: true });
delete process.env.HOME;
});
it("creates credentials.json with mode 0600", async () => {
const { saveCredential } = await import("../../bin/lib/credentials.js");
saveCredential("TEST_KEY", "test-value");
const stat = fs.statSync(path.join(tmpDir, ".nemoclaw", "credentials.json"));
expect(stat.mode & 0o777).toBe(0o600);
});
it("creates .nemoclaw dir with mode 0700", async () => {
const { saveCredential } = await import("../../bin/lib/credentials.js");
saveCredential("TEST_KEY", "test-value");
const stat = fs.statSync(path.join(tmpDir, ".nemoclaw"));
expect(stat.mode & 0o777).toBe(0o700);
});
it("strips carriage returns from values", async () => {
const { saveCredential, getCredential } = await import("../../bin/lib/credentials.js");
saveCredential("TEST_KEY", "value\r\nwith\rcr");
expect(getCredential("TEST_KEY")).toBe("value\nwith\ncr");
});
it("prefers env var over stored credential", async () => {
const { saveCredential, getCredential } = await import("../../bin/lib/credentials.js");
saveCredential("TEST_KEY", "stored-value");
process.env.TEST_KEY = "env-value";
expect(getCredential("TEST_KEY")).toBe("env-value");
delete process.env.TEST_KEY;
});
});Step 4 — Add npm script to package.json:
"scripts": {
"test:coverage": "vitest run --coverage",
"test:coverage:check": "node scripts/check-coverage-ratchet.ts"
}Alternatives Considered
No response
Category
enhancement: feature
Checklist
- I searched existing issues and this is not a duplicate
- This is a design proposal, not a "please build this" request