feat: make V8 JS the primary rule engine; add --js-file; remove -r/--rule#27
Merged
feat: make V8 JS the primary rule engine; add --js-file; remove -r/--rule#27
Conversation
Adds experimental support for JavaScript-based request evaluation using Google's V8 engine via the rusty_v8 crate. This provides more flexible and powerful rule logic compared to regex patterns or shell scripts. Features: - New --js flag for JavaScript rule evaluation - V8 isolate-based execution for security and performance - Global variables: url, method, host, scheme, path - Return true to allow, false to block requests - Fresh context per evaluation for thread safety - Comprehensive error handling for syntax and runtime errors - Full test coverage including unit and integration tests - Updated documentation with examples and performance notes The implementation prioritizes thread safety over performance optimization by creating fresh V8 contexts for each evaluation. This ensures reliable operation in the async, multi-threaded environment while maintaining the security sandbox properties of V8. Conflicts with --script, --rule, and --config flags as only one evaluation method can be active at a time. Co-authored-by: ammario <7416144+ammario@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
Comment on lines
+31
to
+34
| V8_INIT.call_once(|| { | ||
| let platform = v8::new_default_platform(0, false).make_shared(); | ||
| v8::V8::initialize_platform(platform); | ||
| v8::V8::initialize(); |
There was a problem hiding this comment.
[P0] Keep V8 platform alive for entire process
The V8 engine requires the platform object passed to V8::initialize_platform to remain alive for the duration of the program. V8JsRuleEngine::new allocates the platform inside a Once and immediately drops it (lines 31‑34). After the call_once closure returns, the SharedRef’s refcount reaches zero, so V8 continues running with a dangling pointer. Any subsequent isolate creation can crash or exhibit undefined behaviour as soon as JavaScript evaluation is attempted. Store the SharedRef in a static (e.g., OnceLock<SharedRef<Platform>>) so it outlives all isolates before calling initialize_platform.
Useful? React with 👍 / 👎.
- Remove legacy -r/--rule flag and rule config workflows - Add --js-file flag (conflicts with --js) to load JavaScript from file - Make JavaScript evaluation first-class (non-experimental) - Remove PatternRuleEngine; refactor tests to use JS - Update README to document JS and --js-file; remove regex/config docs - Keep script evaluation; default to deny-all when no evaluator provided Co-authored-by: ammario <7416144+ammario@users.noreply.github.com>
blink-so
Bot
changed the title
…move RuleEngine::new and Rule-based tests
…st platform; adjust logs per CLI guidance; resolve merge in tests/common
- Changed JS API from function-based to expression-based evaluation - Before: --js "return host === 'github.com'" - After: --js "r.host === 'github.com'" - Moved all jail variables into 'r' namespace object - Variables now accessed as r.url, r.method, r.host, r.scheme, r.path - Frees up global scope for user-defined variables - Added r.block_message for custom denial messages - Scripts can set r.block_message to provide context in 403 responses - Updated all tests and documentation for new API This makes JS rules more concise and provides better separation between jail-provided variables and user code. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
The test_server_mode test was still using the old regex-based rule syntax (-r flag). Updated to use the new JavaScript expression syntax. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
The CI was incorrectly using --bins flag which doesn't run the library unit tests where the V8 JS tests are located. Changed to --lib to run the actual unit tests. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixed all remaining unit tests that were still using the old function-based JavaScript syntax with 'return' statements. Updated to use the new expression evaluation syntax with 'r.' namespace for variables. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Updated all integration tests to use the new JavaScript expression evaluation syntax with 'r.' namespace. Removed old regex-based rule syntax (-r flag) from concurrent isolation tests. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove 'return' statements from JS expressions in system_integration tests - Replace -r flag with --js in network diagnostic tests - Simplify boolean logic for HTTPS connect tests
- Document how to SSH into ci-1 instance for debugging - Note that only Coder employees have access - Add commands for manual testing on CI
…y files - Scan /etc/netns directory directly for orphaned namespace configs - Handle cases where /tmp is cleaned but namespace configs remain - Avoid duplicate cleanup of same jail ID - Should fix CI connectivity issues from leftover resources
The proxy was binding to 127.0.0.1 by default, making it inaccessible from the veth interface in the network namespace. This caused all connections to be refused in strong jail mode. - Bind to 0.0.0.0 when in strong jail mode (not weak, not server) - Keep localhost binding for weak mode and server mode - This fixes the Linux integration test failures on CI
- Replace old -r regex flag with --js in Linux-specific test - Remove unused mut warning (HashSet only modified on Linux) - All Linux integration tests now passing
The mut is needed on Linux but not on macOS, causing platform-specific clippy warnings. Added #[allow(unused_mut)] attribute to handle this.
Remove 'return' from JS expressions in test_comprehensive_resource_cleanup and test_cleanup_after_sigint tests.
The proxy needs to be accessible from the network namespace's veth interface. Since localhost (127.0.0.1) in the namespace is isolated from the host's localhost, we must bind to an IP reachable from the namespace. Current approach binds to 0.0.0.0 which works but has security implications. Added TODO comment referencing GitHub issue #31 for tracking the proper fix (binding to specific veth IP instead of all interfaces).
- Replace old rule-based examples (-r flag) with JavaScript expressions - Update configuration file examples from rules.txt to rules.js - Show proper JavaScript syntax for regex patterns and method-specific filtering - Maintain consistency with the new V8 JS evaluation approach
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR promotes JavaScript (V8) to the primary rule engine, adds
--js-file, and removes the legacy-r/--rulepattern engine.Key changes:
--js-fileflag to load JS source from a file (conflicts with--js)-r/--ruleand the PatternRuleEngine--script) as an alternative--js-fileDeveloper notes:
If you want me to split this into smaller PRs or retain
-ras a deprecated alias temporarily, say the word and I can adjust.