feat(cli): add --ruleset-file for YAML domain rule configuration#1279
feat(cli): add --ruleset-file for YAML domain rule configuration#1279
Conversation
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 84.37% | 84.62% | 📈 +0.25% |
| Statements | 84.32% | 84.57% | 📈 +0.25% |
| Functions | 84.88% | 84.84% | 📉 -0.04% |
| Branches | 77.40% | 77.41% | ➡️ +0.01% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/cli.ts |
56.3% → 55.7% (-0.60%) | 56.8% → 56.2% (-0.59%) |
src/docker-manager.ts |
86.9% → 87.4% (+0.52%) | 86.2% → 86.7% (+0.50%) |
✨ New Files (1 files)
src/rules.ts: 98.2% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
There was a problem hiding this comment.
Pull request overview
Adds YAML-based domain allowlisting via a new repeatable CLI flag --ruleset-file, enabling users to maintain allowlists as structured files while continuing to support existing --allow-domains / --allow-domains-file inputs.
Changes:
- Introduces
src/rules.tsto load/validate aversion: 1YAML ruleset format and merge rules into allowed domain strings. - Adds Jest coverage for ruleset parsing, validation errors, expansion, and merging (
src/rules.test.ts). - Adds
--ruleset-file <path>(repeatable) to the CLI and merges ruleset-derived domains into the allowlist (src/cli.ts).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/rules.ts | Implements YAML ruleset loading/validation and merging into allowlisted domains. |
| src/rules.test.ts | Adds unit tests for ruleset parsing/validation and merge behavior. |
| src/cli.ts | Adds --ruleset-file option and merges loaded ruleset domains into allowedDomains. |
Comments suppressed due to low confidence (1)
src/cli.ts:1167
loadAndMergeDomains()already returns a de-duplicated list, but the CLI still de-duplicatesallowedDomainsagain immediately after. Consider de-duplicating in only one place (either insideloadAndMergeDomainsor in the CLI) to avoid redundant work and keep responsibilities clearer.
if (options.rulesetFile && Array.isArray(options.rulesetFile) && options.rulesetFile.length > 0) {
try {
allowedDomains = loadAndMergeDomains(options.rulesetFile, allowedDomains);
} catch (error) {
logger.error(`Failed to load ruleset file: ${error instanceof Error ? error.message : error}`);
process.exit(1);
}
}
// Log when no domains are specified (all network access will be blocked)
if (allowedDomains.length === 0) {
logger.debug('No allowed domains specified - all network access will be blocked');
}
// Remove duplicates (in case domains appear in both sources)
allowedDomains = [...new Set(allowedDomains)];
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Expands a single rule into domain strings suitable for the allowedDomains list. | ||
| * | ||
| * When subdomains is true (default), the domain is returned as-is because the | ||
| * existing domain normalization in squid-config.ts and domain-patterns.ts | ||
| * automatically adds subdomain matching (both "example.com" and ".example.com"). | ||
| * | ||
| * When subdomains is false, the domain is prefixed with "exact:" to signal | ||
| * exact-match-only behavior. However, since the current squid config always | ||
| * adds subdomain matching, we return just the bare domain. The subdomain | ||
| * field is reserved for future granular control. | ||
| * | ||
| * @param rule - A single domain rule | ||
| * @returns Array of domain strings | ||
| */ | ||
| export function expandRule(rule: Rule): string[] { | ||
| // The existing system already handles subdomain matching when a plain | ||
| // domain is provided (e.g., "github.com" matches both github.com and | ||
| // *.github.com in Squid config). So we just return the domain. | ||
| return [rule.domain]; | ||
| } |
| ) | ||
| .option( | ||
| '--ruleset-file <path>', | ||
| 'YAML rule file for domain allowlisting (repeatable). Schema: version: 1, rules: [{domain, subdomains}]', |
|
Smoke Test Results
Overall: PASS
|
|
fix(cli): fix secure_getenv() bypass of one-shot token protection
|
|
Smoke Test Results — run 23031227978 ✅ GitHub MCP — Last 2 merged PRs: Overall: PASS | PR author:
|
This comment has been minimized.
This comment has been minimized.
Chroot Version Comparison Results
Result: Not all tests passed. Python and Node.js versions differ between host and chroot environments.
|
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 84.37% | 84.66% | 📈 +0.29% |
| Statements | 84.32% | 84.61% | 📈 +0.29% |
| Functions | 84.88% | 84.84% | 📉 -0.04% |
| Branches | 77.40% | 77.45% | 📈 +0.05% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/cli.ts |
56.3% → 55.7% (-0.60%) | 56.8% → 56.2% (-0.59%) |
src/docker-manager.ts |
86.9% → 87.6% (+0.65%) | 86.2% → 86.9% (+0.63%) |
✨ New Files (1 files)
src/rules.ts: 98.2% lines
Coverage comparison generated by scripts/ci/compare-coverage.ts
Smoke Test Results
Overall: PASS
|
Smoke Test Results — PASS ✅
Author:
|
Chroot Version Comparison Results
Result: ❌ Not all versions match — Python and Node.js versions differ between host and chroot environments.
|
Adds support for YAML rule files via --ruleset-file flag. Rules define domain allowlists with optional subdomain matching. Multiple files can be specified and are merged with --allow-domains. Fixes #136 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add test coverage for hasRateLimitOptions function and --ruleset-file Commander option accumulator to restore Functions coverage metric. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract inline accumulator into named collectRulesetFile function and add direct unit tests to restore Functions coverage metric. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mossaka
force-pushed
the
feat/105-yaml-rules
branch
from
ceb90ee to
12ce6c1
Compare
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
✨ New Files (1 files)
Coverage comparison generated by |
Smoke Test Results
Last 2 merged PRs: #1276 Author: Overall: PASS
|
|
Smoke Test Results — PASS
|
|
feat(cli): add --enable-dind flag to opt-in to Docker socket access
|
Chroot Version Comparison Results
|
🏗️ Build Test Suite Results
Overall: 7/8 ecosystems passed — ❌ FAIL ❌ Failure DetailsJava (gson & caffeine) — Maven compile failed: network unreachable when downloading from Maven Central ( Maven requires internet access to resolve plugins and dependencies.
|
Summary
--ruleset-file <path>CLI option (repeatable) for YAML-based domain allowlistingversion: 1,rulesarray withdomainandsubdomainsfields--allow-domainsand--allow-domains-file, with deduplicationFixes #136
Test plan
npm run buildpassesnpm testpasses (978 tests, 22 suites)npm run lintpasses🤖 Generated with Claude Code