Skip to content

Commit 2062562

Browse files
Add decision effect constants and generateId utility
Addressed comments in PR: - Add EFFECT_ALLOW/EFFECT_DENY constants to DecisionTypes.sys.mjs - Replace hardcoded effect strings across security modules - Add generateId(prefix) utility to SecurityUtils.sys.mjs - Update SmartWindowIntegration and smartwindow.mjs to use generateId
1 parent 6c3784a commit 2062562

File tree

8 files changed

+41
-19
lines changed

8 files changed

+41
-19
lines changed

browser/components/smartwindow/content/smartwindow.mjs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
CHAT_HISTORY_CONVERSATION_SELECTED_EVENT,
1515
} from "chrome://browser/content/smartwindow/chat-history.mjs";
1616
import { deleteInsight, getInsightSummariesForPrompt } from "./insights.mjs";
17+
import { generateId } from "chrome://global/content/ml/security/SecurityUtils.sys.mjs";
1718

1819
const { ChatHistory, ChatHistoryConversation } = ChromeUtils.importESModule(
1920
"resource:///modules/smartwindow/ChatHistory.sys.mjs"
@@ -56,7 +57,7 @@ try {
5657
} catch (e) {
5758
// Actor already registered - this is expected if Smart Window
5859
// has been opened before in this browser session
59-
if (!e.message?.includes("already registered")) {
60+
if (!e.message?.toLowerCase().includes("already registered")) {
6061
console.error("Failed to register SmartWindowMeta actor:", e);
6162
}
6263
}
@@ -116,7 +117,7 @@ class SmartWindowPage {
116117
// Initialize security orchestrator (if enabled)
117118
// Creates the SessionLedger that tracks trusted URLs across tabs
118119
// Only initialize if Smart Window security is enabled
119-
const sessionId = `smart-window-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`;
120+
const sessionId = generateId("smart-window");
120121
const securityEnabled = Services.prefs.getBoolPref("browser.smartwindow.security.enabled", true);
121122

122123
if (securityEnabled) {

browser/components/smartwindow/content/utils.mjs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ ChromeUtils.defineESModuleGetters(lazy, {
1818
import { createEngine } from "chrome://global/content/ml/EngineProcess.sys.mjs";
1919
import { SmartAssistEngine } from "moz-src:///browser/components/genai/SmartAssistEngine.sys.mjs";
2020
import { deleteInsight, findRelatedInsight, generateInsightsFromDirectChat } from "chrome://browser/content/smartwindow/insights.mjs";
21+
import { EFFECT_DENY } from "chrome://global/content/ml/security/DecisionTypes.sys.mjs";
2122

2223
const { ChatHistoryMessage } = ChromeUtils.importESModule(
2324
"resource:///modules/smartwindow/ChatHistory.sys.mjs"
@@ -602,7 +603,7 @@ async function checkToolSecurity(toolName, toolParams, requestId) {
602603
},
603604
});
604605

605-
if (decision.effect === "deny") {
606+
if (decision.effect === EFFECT_DENY) {
606607
return {
607608
allowed: false,
608609
reason: `Security policy blocked this action: ${decision.reason} (${decision.code})`,

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ export class SecurityPolicyError extends Error {
9898
}
9999
}
100100

101+
// Decision effect constants
102+
export const EFFECT_ALLOW = "allow";
103+
export const EFFECT_DENY = "deny";
104+
101105
// Standard denial codes for consistent error handling across the security layer.
102106
export const DenialCodes = Object.freeze({
103107
// URL not present in the request-scoped link ledger.
@@ -131,7 +135,7 @@ export const ReasonPhrases = Object.freeze({
131135
*/
132136
export const allow = () =>
133137
/** @type {SecurityDecisionAllow} */ ({
134-
effect: "allow",
138+
effect: EFFECT_ALLOW,
135139
});
136140

137141
/**
@@ -150,7 +154,7 @@ export const deny = (
150154
policyId = "block-unseen-links"
151155
) =>
152156
/** @type {SecurityDecisionDeny} */ ({
153-
effect: "deny",
157+
effect: EFFECT_DENY,
154158
policyId,
155159
code,
156160
reason,
@@ -163,12 +167,12 @@ export const deny = (
163167
* @param {SecurityDecision | undefined | null} decision - Decision to check
164168
* @returns {boolean} True if decision is allow
165169
*/
166-
export const isAllow = decision => decision?.effect === "allow";
170+
export const isAllow = decision => decision?.effect === EFFECT_ALLOW;
167171

168172
/**
169173
* Type guard: checks if a decision is a deny.
170174
*
171175
* @param {SecurityDecision | undefined | null} decision - Decision to check
172176
* @returns {boolean} True if decision is deny
173177
*/
174-
export const isDeny = decision => decision?.effect === "deny";
178+
export const isDeny = decision => decision?.effect === EFFECT_DENY;

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
import {
6+
EFFECT_ALLOW,
7+
EFFECT_DENY,
68
allow,
79
deny,
810
} from "chrome://global/content/ml/security/DecisionTypes.sys.mjs";
@@ -78,7 +80,7 @@ export class PolicyEvaluator {
7880
* @param {boolean} policy.enabled - Whether policy is active
7981
* @param {object} policy.match - Match criteria
8082
* @param {Array} policy.conditions - Conditions to evaluate
81-
* @param {string} policy.effect - "deny" or "allow"
83+
* @param {string} policy.effect - EFFECT_DENY or EFFECT_ALLOW
8284
* @param {object} policy.onDeny - Denial information
8385
* @param {object} action - Action being evaluated
8486
* @param {object} context - Request context
@@ -97,7 +99,7 @@ export class PolicyEvaluator {
9799
const result = ConditionEvaluator.evaluate(condition, action, context);
98100

99101
if (!result) {
100-
if (policy.effect === "deny") {
102+
if (policy.effect === EFFECT_DENY) {
101103
return deny(policy.onDeny.code, policy.onDeny.reason, {
102104
policyId: policy.id,
103105
failedCondition: condition.type,
@@ -112,7 +114,7 @@ export class PolicyEvaluator {
112114
}
113115
}
114116

115-
if (policy.effect === "deny") {
117+
if (policy.effect === EFFECT_DENY) {
116118
return null;
117119
}
118120

@@ -152,7 +154,7 @@ export class PolicyEvaluator {
152154

153155
appliedPolicies++;
154156

155-
if (decision.effect === "deny") {
157+
if (decision.effect === EFFECT_DENY) {
156158
console.warn(
157159
`[PolicyEvaluator] Policy ${policy.id} denied action:`,
158160
decision.reason
@@ -210,12 +212,12 @@ export class PolicyEvaluator {
210212
if (!Array.isArray(policy.conditions)) {
211213
errors.push("Field 'conditions' must be an array");
212214
}
213-
if (policy.effect !== "deny" && policy.effect !== "allow") {
215+
if (policy.effect !== EFFECT_DENY && policy.effect !== EFFECT_ALLOW) {
214216
errors.push("Field 'effect' must be 'deny' or 'allow'");
215217
}
216218

217219
// Conditional requirements
218-
if (policy.effect === "deny" && !policy.onDeny) {
220+
if (policy.effect === EFFECT_DENY && !policy.onDeny) {
219221
errors.push("Field 'onDeny' required when effect is 'deny'");
220222
}
221223
if (policy.onDeny && (!policy.onDeny.code || !policy.onDeny.reason)) {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5+
import { EFFECT_DENY } from "chrome://global/content/ml/security/DecisionTypes.sys.mjs";
6+
57
const logConsole = console.createInstance({
68
maxLogLevelPref: "browser.ml.logLevel",
79
prefix: "SecurityLogger",
@@ -33,7 +35,7 @@ export class SecurityLogger {
3335
`[${phase}] Security evaluation error:`,
3436
error.message || error
3537
);
36-
} else if (decision.effect === "deny") {
38+
} else if (decision.effect === EFFECT_DENY) {
3739
logConsole.warn(
3840
`[${phase}] DENY: ${decision.code} - ${decision.reason} (${durationMs}ms)`
3941
);

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import { SessionLedger } from "chrome://global/content/ml/security/SecurityUtils.sys.mjs";
66
import { SecurityLogger } from "chrome://global/content/ml/security/SecurityLogger.sys.mjs";
77
import {
8+
EFFECT_ALLOW,
89
allow,
910
deny,
1011
} from "chrome://global/content/ml/security/DecisionTypes.sys.mjs";
@@ -221,13 +222,13 @@ export class SecurityOrchestrator {
221222
action,
222223
context,
223224
decision: {
224-
effect: "allow",
225+
effect: EFFECT_ALLOW,
225226
reason: "Security disabled via kill switch",
226227
},
227228
durationMs: Date.now() - startTime,
228229
killSwitchBypass: true,
229230
});
230-
return { effect: "allow" };
231+
return { effect: EFFECT_ALLOW };
231232
}
232233

233234
const policies = this.#policies.get(phase);

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* - eTLD+1 (effective top-level domain) validation
1111
* - TabLedger: Per-tab trusted URL storage
1212
* - SessionLedger: Container for all tab ledgers in a Smart Window session
13+
* - Unique ID generation (given a prefix)
1314
*
1415
* Security Model:
1516
* ---------------
@@ -436,3 +437,13 @@ export class SessionLedger {
436437
return this.tabs.size;
437438
}
438439
}
440+
441+
/**
442+
* Generates a unique ID with the given prefix.
443+
*
444+
* @param {string} prefix - Prefix for the ID (e.g., "req", "session")
445+
* @returns {string} Unique ID like "req-1234567890-abc123def"
446+
*/
447+
export function generateId(prefix) {
448+
return `${prefix}-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`;
449+
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
import { SessionLedger } from "chrome://global/content/ml/security/SecurityUtils.sys.mjs";
5+
import { SessionLedger, generateId } from "chrome://global/content/ml/security/SecurityUtils.sys.mjs";
66

77
const logConsole = console.createInstance({
88
maxLogLevelPref: "browser.ml.logLevel",
@@ -27,7 +27,7 @@ let gSessionId = null;
2727
*/
2828
export function initSecurityLayer(sessionId = null) {
2929
if (!sessionId) {
30-
sessionId = `smart-window-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`;
30+
sessionId = generateId("smart-window");
3131
}
3232

3333
gSessionId = sessionId;
@@ -176,7 +176,7 @@ export function getRequestContext(
176176
});
177177

178178
const actualRequestId =
179-
requestId || `req-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`;
179+
requestId || generateId("req");
180180

181181
return {
182182
linkLedger,

0 commit comments

Comments
 (0)