[K9VULN-14008] Fix stale v8 script cache when using `datadog-static-analyzer-server` by jasonforal · Pull Request #901 · DataDog/datadog-static-analyzer

jasonforal · 2026-04-28T15:22:39Z

What problem are you trying to solve?

When running datadog-static-analyzer-server with the --use-rules-cache CLI argument, there are two layers of cache, both keyed by the rule's name:

(Enabled by the flag): A cache for the RuleInternal on the datadog-static-analyzer-server, which exists primary to cache the compiled tree-sitter query.
(Already existing): A cache for the precompiled v8 script (Javascript code) on the JsRuntime.

The cache is designed such that the caller is expected to to manage the JsRuntime's v8 script cache. Currently, the datadog-static-analyzer-server explicitly evicts the v8 script cache for a specific rule when the incoming JSON payload represents a separate rule:

datadog-static-analyzer/crates/bins/src/bin/datadog_static_analyzer_server/rule_cache.rs

Lines 99 to 102 in b118e6e

    
           if cached.is_update { 
        
               // The runtime's `v8::Script` cache is keyed only by name -- we need to manually clear it. 
        
               runtime.clear_rule_cache(&cached.rule.inner().name); 
        
           }

This scenario essentially only occurs when working on a custom rule (where each edit to the JavaScript or tree-sitter query sends a brand new rule).

The issue is that there are multiple JsRuntime -- one for each thread:

datadog-static-analyzer/crates/bins/src/bin/datadog_static_analyzer_server/endpoints.rs

Lines 128 to 131 in b118e6e

    
           thread_local! { 
        
               // (`Cell` is used to allow lazy instantiation of a thread local with zero runtime cost). 
        
               static JS_RUNTIME: Cell<Option<JsRuntime>> = const { Cell::new(None) }; 
        
           }

But there is only a single RuleInternal cache, shared across all threads:

datadog-static-analyzer/crates/bins/src/bin/datadog-static-analyzer-server.rs

Line 14 in b118e6e

pub(crate) static RULE_CACHE: OnceLock<RuleCache> = OnceLock::new();

The issue is that while the is_update boolean works correctly, it is only ever triggered once. Because the cached_analysis_request accepts the JsRuntime as an argument, this means that only a single runtime's v8 script cache is cleared. Thus, if the server is run with multiple threads, all other threads will continue to have a stale cache.

What is your solution?

Track the hash of the rule code associated with the v8 script within the JsRuntime itself. Within datadog-static-analyzer-server, always hash and check equivalence of rule code.
- Note: this does introduce overhead for the overwhelming majority of cases (i.e. normal IDE usage where rules don't change), though sha256 hashing is generally hardware-accelerated and should have minimal impact.

Alternatives considered

Introduce a query param to the server's /analyze endpoint and thread this through to cached_analysis_request, such that the server would only hash and check the v8 script cache if that query param is present. Then, the VS Code extension would modify only the custom rule editor to pass this query param. This would be a "correct" and surgical implementation that avoids unnecessary hashing.
- This would require coordination across multiple codebases. I'd rather only resort to this if benchmarks show this PR introduces a noticeable regression. I don't expect it will.

What the reviewer should know

This bug only really surfaces in the VS Code extension's custom rule editor and thus doesn't affect correctness of standard IDE usage.
Beyond multiple runtimes being a blind spot, this issue also slipped by because there was no actual test coverage on the v8 script cache (despite a test comment misleading saying there was). This has been addressed with this update.

…undScript. Add helper to evict the cache if this hash doesn't match

…ting the v8::UnboundScript cache for the passed in JSRuntime.

Copilot

Pull request overview

Fixes stale v8 script caching in datadog-static-analyzer-server when rule names collide across updates and multiple thread-local JsRuntimes are in use, by making runtime-side script cache eviction dependent on rule code identity rather than a one-time “is_update” signal.

Changes:

Add a per-script code fingerprint to the kernel JsRuntime rule-script cache and introduce evict_script_if_stale(rule_name, rule_code).
Simplify server RuleCache API (remove CachedValue/is_update) and always ask each JsRuntime to evict stale scripts by code hash before execution.
Add/adjust tests to cover v8 UnboundScript cache eviction behavior, including multi-runtime simulation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`crates/static-analysis-kernel/src/analysis/ddsa_lib/runtime.rs`	Stores a rule-code fingerprint alongside cached compiled scripts and exposes stale-eviction API + unit test.
`crates/bins/src/bin/datadog_static_analyzer_server/rule_cache.rs`	Updates server rule caching flow to evict stale runtime scripts by code hash and expands test coverage (including multi-runtime case).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

datadog-prod-us1-5 · 2026-04-28T15:28:05Z

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 85.09% (+0.06%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: c7ebf93 | Docs | Datadog PR Page | Give us feedback!}

bahar-shah · 2026-04-28T16:34:40Z

    /// Panics if the `rule_cache` has an existing borrow.
-    pub fn clear_rule_cache(&self, rule_name: &str) -> bool {
-        self.rule_cache.borrow_mut().remove(rule_name).is_some()
+    pub fn evict_script_if_stale(&self, rule_name: &str, rule_code: &str) -> bool {


This could avoid holding a mutable borrow when no eviction is needed:

pub fn evict_script_if_stale(&self, rule_name: &str, rule_code: &str) -> bool { // First check with read-only access if let Some(entry) = self.rule_cache.borrow().get(rule_name) { if entry.code_hash == CompiledRule::get_code_hash(rule_code) { return false; // No eviction needed, never took mut borrow } } else { return false; } // Only take mut borrow if eviction is actually needed self.rule_cache.borrow_mut().remove(rule_name).is_some() }

This reduces lock contention in the common case where rules haven't changed.

There will never be lock contention, otherwise this would actually panic. We're using an Rc<RefCell<_>> here, not a mutex. It's governed by runtime borrow checking.

bahar-shah · 2026-04-28T16:36:45Z

+        let Some(entry) = cache.get(rule_name) else {
+            return false;
+        };
+        if entry.code_hash != CompiledRule::get_code_hash(rule_code) {


get_code_hash(rule_code) is called on every request, even for cache hits. not sure if this is a concern from a performance perspective or if the overhead here is negligible. You could otherwise precompute it in RuleInternal during construction and pass it in but that might be overkill tbh.

This is mentioned in the description

bahar-shah · 2026-04-28T16:37:44Z

-            }
-            compiled_rules.push(cached.rule);
+            let internal = cached.as_internal();
+            let _ = runtime.evict_script_if_stale(&internal.name, &internal.code);


nit- the return value isn't used here. can we remove it?

It's essentially a style question in this case. I just prefer to be explicit

jasonforal added 2 commits April 27, 2026 19:30

Store the hash of the original code that was compiled into a v8::Unbo…

9bd8970

…undScript. Add helper to evict the cache if this hash doesn't match

Fix bug where the cached_analysis_request was not properly invalida…

d2fbb08

…ting the v8::UnboundScript cache for the passed in JSRuntime.

Copilot AI review requested due to automatic review settings April 28, 2026 15:22

jasonforal requested a review from a team as a code owner April 28, 2026 15:22

jasonforal requested review from bahar-shah and removed request for a team April 28, 2026 15:22

Copilot started reviewing on behalf of jasonforal April 28, 2026 15:23 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

Comment thread crates/bins/src/bin/datadog_static_analyzer_server/rule_cache.rs

Comment thread crates/static-analysis-kernel/src/analysis/ddsa_lib/runtime.rs

Comment thread crates/static-analysis-kernel/src/analysis/ddsa_lib/runtime.rs

Fix partially-hardcoded test helper

c7ebf93

bahar-shah approved these changes Apr 28, 2026

View reviewed changes

jasonforal merged commit 8fdfae4 into main Apr 28, 2026
90 checks passed

jasonforal deleted the jf/K9VULN-14008 branch April 28, 2026 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[K9VULN-14008] Fix stale v8 script cache when using `datadog-static-analyzer-server`#901

[K9VULN-14008] Fix stale v8 script cache when using `datadog-static-analyzer-server`#901
jasonforal merged 3 commits intomainfrom
jf/K9VULN-14008

jasonforal commented Apr 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

datadog-prod-us1-5 Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

bahar-shah Apr 28, 2026

Uh oh!

jasonforal Apr 28, 2026

Uh oh!

bahar-shah Apr 28, 2026

Uh oh!

jasonforal Apr 28, 2026

Uh oh!

bahar-shah Apr 28, 2026

Uh oh!

jasonforal Apr 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if cached.is_update {
	// The runtime's `v8::Script` cache is keyed only by name -- we need to manually clear it.
	runtime.clear_rule_cache(&cached.rule.inner().name);
	}

	thread_local! {
	// (`Cell` is used to allow lazy instantiation of a thread local with zero runtime cost).
	static JS_RUNTIME: Cell<Option<JsRuntime>> = const { Cell::new(None) };
	}

Conversation

jasonforal commented Apr 28, 2026

What problem are you trying to solve?

What is your solution?

Alternatives considered

What the reviewer should know

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

datadog-prod-us1-5 Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bahar-shah Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

jasonforal Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

bahar-shah Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

jasonforal Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

bahar-shah Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

jasonforal Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

datadog-prod-us1-5 Bot commented Apr 28, 2026 •

edited

Loading