[GENAI-2335] Implement Policy Engine for Smart Window tool execution by randy-concepcion · Pull Request #124 · Firefox-AI/firefox-prototypes

randy-concepcion · 2025-11-24T15:52:08Z

Summary

Implements the Phase 2 POC security layer with policy enforcement for Smart Window tool execution. Enforces a single policy (block-unseen-links) that prevents tools from accessing URLs not in the trusted page context, protecting against data exfiltration via prompt injection.

Changes

New modules (toolkit/components/ml/security/):

SecurityOrchestrator.sys.mjs — Central coordination with kill switch
PolicyEvaluator.sys.mjs / ConditionEvaluator.sys.mjs — JSON-based policy evaluation
SecurityUtils.sys.mjs — URL normalization and session-scoped ledgers
DecisionTypes.sys.mjs — Structured allow/deny decisions
SecurityLogger.sys.mjs — Audit logging (stub)

Smart Window integration (browser/components/smartwindow/):

SmartWindowMetaChild/Parent actors — Page metadata extraction (canonical/og:url)
utils.mjs — Tool dispatch integration via checkToolSecurity()

Policy (policies/tool-execution-policies.json):

block-unseen-links — Blocks tool calls to URLs not in trusted page context

Security Model

Explicit seeding: AI only accesses URLs when user explicitly requests them
Deterministic validation: Exact URL matching against session ledger
Fail-closed: Security errors deny tool execution
Kill switch: browser.smartwindow.security.enabled preference

Testing

Unit tests for policy evaluation, URL normalization, ledger management
Integration tests for allowed/denied scenarios, kill switch, fail-closed behavior

Screenshots

Security Layer + policy loaded up with Smart Window

Accessing untrusted link and enforced with deny action

Accessing trusted link and validated with allow action

browser/components/smartwindow/content/smartwindow.mjs

toolkit/components/ml/security/ConditionEvaluator.sys.mjs

toolkit/components/ml/security/PolicyEvaluator.sys.mjs

toolkit/components/ml/security/SmartWindowIntegration.sys.mjs

noahpodgurski · 2025-11-24T22:06:33Z

Also https://mozilla.slack.com/archives/C09AR1FJCJ1/p1763724678792239 🫠

randy-concepcion · 2025-11-25T18:43:45Z

Also https://mozilla.slack.com/archives/C09AR1FJCJ1/p1763724678792239 🫠

Yes, thanks for the reminder about the name change 😅 I'll make updates to the files/names in a separate commit 😬 (as well as the refactor in a separate commit for the bug you found above!).

toolkit/components/ml/security/DecisionTypes.sys.mjs

…nent for LLM calls and tool dispatch. r=tarek,ai-ondevice-reviewers - Moved JSActor pattern from security-specific JSActor back to parent MLEngine JSActor - Move validation logic for both request/response from child to parent to avoid IPC round trip (improve performance) Differential Revision: https://phabricator.services.mozilla.com/D271495

Add security layer policy enforcement for Smart Window tool execution with policy to prevent exfiltration links Components: - SecurityOrchestrator: Central coordination with pref switch - PolicyEvaluator/ConditionEvaluator: JSON-based policy evaluation - SecurityUtils: URL normalization and session-scoped ledgers - SmartWindowMeta actors: Page metadata extraction (canonical/og:url) - DecisionTypes: Structured allow/deny decisions Security model: Explicit seeding with deterministic URL validation, fail-closed behavior, and pref switch (browser.smartwindow.security.enabled).

Addressed comments in PR: - Add EFFECT_ALLOW/EFFECT_DENY constants to DecisionTypes.sys.mjs - Add PolicyEffect typedef for type hints - Replace hardcoded effect strings across security modules - Add generateId(prefix) utility to SecurityUtils.sys.mjs - Update SmartWindowIntegration and smartwindow.mjs to use generateId

randy-concepcion · 2025-11-30T22:13:06Z

I split out the security layer-related code to land in mozilla-central . This is separate from the AI Window integration. The Phab change can be found here: https://phabricator.services.mozilla.com/D274523.

- Fixes multi-window bug where closing one AI Window would reset security state for all windows. - Fixes minor bug for allowedUrls and other lint errors - Update SecurityOrchestrator instantiation in xpcshell tests - Remove deprecated files and tests (SmartWindowIntegration.sys.mjs)

noahpodgurski · 2025-12-02T20:43:12Z

browser/components/smartwindow/content/utils.mjs

+  if (added) {
+    console.log(`[Security] Seeded @mentioned URL: ${url} for tab ${tabId}`);
+  } else {
+    console.warn(`[Security] Failed to seed @mentioned URL: ${url} for tab ${tabId}`);


question: Would it make sense for this to be considered an error and brought to the user's attention that the permission access request failed?

Great question! I've given this some thought. Since these log messages are intended for developers rather than users, I don't think we need to surface this to the user directly. They'll see a denial when the tool call is evaluated by the security layer (fail-closed behavior).

However, I've tightened up the logging to give better signal: "no ledger available" is now console.error, while "already in ledger" is console.debug. This should give developers better visibility into the unexpected failures.

noahpodgurski · 2025-12-02T21:00:14Z

toolkit/components/ml/security/SecurityOrchestrator.sys.mjs

   */
-  static async #loadPolicies() {
-    // Add more policy files here as they're created:
+  async #loadPolicies() {


thought: This is probably not necessary but a nice optimization might be to keep this method static, add a static loadedPolicies = null attribute, and only call this in the create method if loadedPolicies is null? Something like

if (SecurityOrchestrator.#loadedPolicies === null) { SecurityOrchestrator.#loadedPolicies = SecurityOrchestrator.#loadPolicies(); } this.#policies = SecurityOrchestrator.#loadedPolicies

But if loadPolicies isn't very expensive to run it might not be worth it

Good call on the optimization 🙏 Since I'll need to update this and all the test files, I'll defer this optimization in this POC for now. The next phase, I'll be refactoring the orchestrator to a singleton pattern where caching should happen naturally at the service level!

tarekziade

pretty nice work thanks;

I think it's worth documenting the flow in the top level class

also, not sure about using classes as pure namespaces, and I also wonder about introducing a new child process just for extracting the metadata. I'd like to get clarity on what you have in mind in terms of IPC for the longer term

tarekziade · 2025-12-03T10:41:48Z

browser/components/smartwindow/actors/SmartWindowMetaChild.sys.mjs

+   *
+   * @returns {object} Metadata with pageUrl, canonical, and ogUrl (raw strings)
+   */
+  getMetadata() {


i feel like we probably already have an existing API for this

Yes, good point. I'll research more if there's an existing API I can use for this. 🕵️

tarekziade · 2025-12-03T10:43:13Z

browser/components/smartwindow/actors/SmartWindowMetaParent.sys.mjs

+    };
+
+    try {
+      const metadata = await this.sendQuery("SmartWindowMeta:GetMetadata");


what is the incentive to run this in the child process?

Good question! My understanding is that the parent process wouldn't be able to query the DOM directly (i.e., document.querySelector() ) since the DOM lives in the child process.

However, I should definitely look into the existing API you mentioned before that would already handle this 😅

tarekziade · 2025-12-03T10:44:52Z

browser/components/smartwindow/actors/SmartWindowMetaParent.sys.mjs

+
+      const { pageUrl, canonical, ogUrl } = metadata;
+
+      const normalizedPageUrl = normalizeUrl(pageUrl);


more page info data extraction here, I feel like this belong to the previous function

Good catch! I can consolidate this!

tarekziade · 2025-12-03T10:46:30Z

browser/components/smartwindow/content/utils.mjs

+            } else {
+              const result = await actor.seedLedgerForTab(sessionLedger, tabId);
+
+              if (result.success) {


we're not doing anything with result.error, so you will silent problems

Ah, nice catch. I'll add the appropriate logging here. 💯

tarekziade · 2025-12-03T13:05:24Z

browser/components/smartwindow/content/utils.mjs

+                let targetTab = gBrowser.tabs.find(tab => {
+                  const tabUrl = tab.linkedBrowser.currentURI.spec;
+                  return (
+                    tabUrl === toolParams.url ||


add a comment here on what you are doing

Good call, I'll add a comment to explain the logic. 💯

tarekziade · 2025-12-03T13:10:03Z

toolkit/components/ml/security/ConditionEvaluator.sys.mjs

+ * Safe condition evaluator for JSON-based security policies.
+ * Evaluates policy conditions against action and context.
+ */
+export class ConditionEvaluator {


what's the point of having a class here if all methods are static?
if it's for the namespace you can do this directly when exporting

Ah, yes. I had started writing these as classes and just kept the pattern, but you're right with exports making more sense. Thanks for the recommendation, I'll refactor this!

tarekziade · 2025-12-03T13:11:24Z

toolkit/components/ml/security/PolicyEvaluator.sys.mjs

+ * Evaluates JSON-based security policies using "first deny wins" strategy.
+ * Delegates condition evaluation to ConditionEvaluator.
+ */
+export class PolicyEvaluator {


same question for using a class just for the namespace

+1 I'll refactor this!

tarekziade · 2025-12-03T13:11:57Z

toolkit/components/ml/security/PolicyEvaluator.sys.mjs

+   * @returns {boolean} True if policy applies to this action
+   */
+  static checkMatch(matchCriteria, action) {
+    console.warn(


why warn here? shouln't it be debug?

Yes, that makes sense. I will change this to debug 🐛

Ah, right, I remembered why I changed this to warn: the linter doesn't like this :( When I run ./mach lint, I get the following lint error:

34:5 error Unexpected console statement. Only these console methods are allowed: createInstance, error, warn. no-console (eslint)

For now, I can add the comment to disable the lint error since this is a log intentionally for debugging purposes.

tarekziade · 2025-12-03T13:14:21Z

toolkit/components/ml/security/PolicyEvaluator.sys.mjs

+      }
+    }
+
+    if (policy.effect === EFFECT_DENY) {


can't this be placed before the for loop and short cur ? I am not sure to follow the logic

Really great catch here. The flow here is a bit hard to follow after looking at this again 🤔 Your suggestion makes sense, I can refactor this!

tarekziade · 2025-12-03T13:15:06Z

toolkit/components/ml/security/SecurityLogger.sys.mjs

+ * Security audit logger (stub - console only).
+ * TODO: Full implementation in separate ticket (NDJSON, Glean, field hashing).
+ */
+export class SecurityLogger {


why using a class?

+1 I'll changes this as mentioned in previous comments 👍

- Security logs errors on no ledger available + debug if already in ledger - Reorganized imports and security functions

- Consolidate URL processing into #processMetadataUrls() in SmartWindowMetaParent - Add console.warn when security policy blocks tool execution - Add comment explaining targetTab URL matching logic - Add default case to tool switch

randy-concepcion · 2025-12-04T05:07:14Z

pretty nice work thanks;

I think it's worth documenting the flow in the top level class

also, not sure about using classes as pure namespaces, and I also wonder about introducing a new child process just for extracting the metadata. I'd like to get clarity on what you have in mind in terms of IPC for the longer term

Thanks again for your feedback, @tarekziade! 🙏 Here's a summary of the changes I've made:

Phabricator Revisions (ready for review)
These are ready for review to land in mozilla-central . Afterwards, they can get pulled into this PR with the AI Window integrations in browser/components/smartwindow.

Policy Engine: https://phabricator.services.mozilla.com/D274523
- Added evaluation flow documentation to SecurityOrchestrator
- Refactored ConditionEvaluator, PolicyEvaluator, SecurityLogger from static classes to plain module exports
- Refactored evaluatePolicy() for clearer control flow
- Changed console.warn to console.debug in PolicyEvaluator.checkMatch
Security Logger: https://phabricator.services.mozilla.com/D274680
- Updated test imports to match the module export refactoring

Changes For This PR

Consolidated URL processing into #processMetadataUrls() in SmartWindowMetaParent
Added console.warn when security policy blocks tool execution
Added comment explaining targetTab URL matching logic
Added default case to tool switch for defense-in-depth

Metadata API: I'm still looking into using an existing API instead of the SmartWindowMeta actor. I'll follow up once I've investigated.

randy-concepcion force-pushed the GENAI-2335-policy-engine branch from 9574528 to 6e9dc59 Compare November 24, 2025 16:44

randy-concepcion changed the title ~~[WIP] [GENAI-2335] Implement Policy Engine for Smart Window tool execution~~ [GENAI-2335] Implement Policy Engine for Smart Window tool execution Nov 24, 2025

randy-concepcion marked this pull request as ready for review November 24, 2025 16:48

noahpodgurski reviewed Nov 24, 2025

View reviewed changes

noahpodgurski reviewed Nov 25, 2025

View reviewed changes

toolkit/components/ml/security/DecisionTypes.sys.mjs Show resolved Hide resolved

randy-concepcion force-pushed the GENAI-2335-policy-engine branch 2 times, most recently from 2062562 to ce28b69 Compare November 25, 2025 23:15

randy-concepcion added 3 commits November 28, 2025 15:12

randy-concepcion force-pushed the GENAI-2335-policy-engine branch 2 times, most recently from 597e9af to 55c9c0d Compare November 30, 2025 04:50

randy-concepcion force-pushed the GENAI-2335-policy-engine branch from 55c9c0d to 3f76c54 Compare November 30, 2025 23:39

noahpodgurski reviewed Dec 2, 2025

View reviewed changes

tarekziade reviewed Dec 3, 2025

View reviewed changes

randy-concepcion added 2 commits December 3, 2025 10:52

Improve security logger message in utils + reorg code

bf8d795

- Security logs errors on no ledger available + debug if already in ledger - Reorganized imports and security functions


		const { pageUrl, canonical, ogUrl } = metadata;

		const normalizedPageUrl = normalizeUrl(pageUrl);

Conversation

randy-concepcion commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Security Model

Testing

Screenshots

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

noahpodgurski commented Nov 24, 2025

Uh oh!

randy-concepcion commented Nov 25, 2025

Uh oh!

Uh oh!

randy-concepcion commented Nov 30, 2025

Uh oh!

noahpodgurski Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarekziade left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

randy-concepcion commented Dec 4, 2025

Uh oh!

Reviewers

Assignees

randy-concepcion commented Nov 24, 2025 •

edited

Loading

noahpodgurski Dec 2, 2025 •

edited

Loading