Actionable comments posted: 4

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/src/services/functions/function.service.ts`:
- Around line 364-374: The current /Deno\b/i pattern blocks any use of Deno
including the allowed Deno.env.get() secrets API; update the dangerousPatterns
array to only block dangerous Deno APIs while allowing Deno.env.get calls (used
by functions/worker-template.js). Replace the single /Deno\b/i entry with a
targeted pair such as /Deno\.(?!env\s*\.\s*get)/i to catch property access other
than env.get and an additional /\bDeno\b\s*(?!\.\s*env\s*\.\s*get)/i to prevent
bare/global Deno usage except when followed by .env.get; keep the rest of
dangerousPatterns unchanged. Ensure the updated patterns preserve
case-insensitivity and the existing check logic that uses dangerousPatterns in
the function validation flow.

In `@docker-compose.yml`:
- Line 191: Replace the insecure Deno invocation that uses --allow-all for
running functions/server.ts with scoped permissions: remove --allow-all and add
--allow-env, --allow-net, and --allow-read so the parent runtime (the process
that reads secrets like JWT_SECRET/ENCRYPTION_KEY and database credentials and
calls Deno.env.get, Deno.readTextFile, and Deno.serve in functions/server.ts)
only has the least-privilege permissions required.

In `@functions/server.ts`:
- Around line 167-170: The permissions block in functions/server.ts currently
sets net: false which blocks the injected SDK's createClient() from calling the
internal backend used by functions/examples/demo-whoami.js; update the
permissions object (permissions) to keep external networking disabled but allow
the internal backend hosts used by INSFORGE_INTERNAL_URL (e.g.,
http://insforge:7130 and http://localhost:7130 as defined in
backend/src/utils/seed.ts and backend/src/utils/environment.ts) so the injected
createClient() can reach the backend while external egress remains blocked.

In `@functions/worker-template.js`:
- Around line 39-49: The mockDeno currently spreads globalThis.Deno and exposes
env.toObject(), which leaks APIs and secrets; update the mockDeno to not use
...globalThis.Deno (remove the spread), expose only the minimal APIs needed
(keep env.get), remove env.toObject entirely, and explicitly block subprocess
APIs by adding stubbed/throwing implementations for Deno.Command in addition to
run and spawn so any attempt to invoke subprocesses via Deno.Command/run/spawn
throws an error; reference the mockDeno object and the env.get, env.toObject,
run, spawn, and Deno.Command symbols when making these changes.

Actionable comments posted: 2

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@functions/server.ts`:
- Line 168: The env allow-list in functions/server.ts (the env array passed into
the worker/process spawn) is too broad and exposes host/container details;
reduce it to the minimal variables the worker template actually needs (e.g.,
keep only 'NODE_ENV' and 'DEBUG' and add 'PORT' only if the worker depends on
it) and remove 'USER', 'PATH', 'PWD', 'HOSTNAME', 'TMPDIR', 'TEMP', 'TMP' to
avoid leaking fingerprint info (the worker template already shadows process.env
and mocks Deno.env.get()). Also reformat the env array into multiple wrapped
lines to satisfy Prettier and CI; update the env array declaration where it
appears in server.ts accordingly.
- Around line 164-175: The Worker is created with a deno.permissions object (new
Worker(..., { deno: { permissions: { ... } } })), but Deno only honors that when
started with --unstable-worker-options; update the production Deno startup
command used for container deployment to include the --unstable-worker-options
flag so the deno.permissions on Worker instances is respected and worker
permission isolation is enforced; ensure the prod service's deno run command
mirrors the development config which already includes --unstable-worker-options.

Actionable comments posted: 1

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/src/services/functions/function.service.ts`:
- Around line 364-373: The current entry-point validation in dangerousPatterns
misses patterns like Deno.Command, bracketed template access (Deno[`run`]), and
dynamic Function constructors; update the dangerousPatterns array (the array
named dangerousPatterns in function.service.ts) to include regexes that match
new\s+Deno\.Command\b, \bDeno\.(Command|run)\b,
Deno\[\s*[`'"]\s*run\s*[`'"]\s*\], and Function\s*\( / new\s+Function\b (or
equivalent) so payloads like new Deno.Command(...), Deno[`run`]({...}) and
Function('return this')() are detected and rejected before storage; then add
regression tests that assert rejection for the three inputs shown (new
Deno.Command('sh', { args: ['-c','id'] }); Deno[`run`]({ cmd: ['id'] });
Function('return this')();) to prevent regressions.

Code Review — #1137: Two-Tier Sandbox Isolation

Summary: The defense-in-depth approach (regex gatekeeper + native Worker permissions) is the right architectural direction for closing the RCE gap, and the test file is a meaningful addition. However, four critical issues block merging: two broken regex patterns that will reject most legitimate function code, a Worker permission misconfiguration that undermines the "Secret Shielding" claim, and a severe dev-environment regression.

Requirements context: No spec or plan under /docs/superpowers/specs/ or /docs/superpowers/plans/ matches this PR's scope. Assessed against PR title, body, and linked issue #1104 alone.

Findings

Critical

[Software Engineering / Functionality] /self/i regex has no word boundary — massive false positives

backend/src/services/functions/function.service.ts:366

The pattern /self/i matches the substring self anywhere inside any identifier — including myself, itself, selfRef, isSelected, etc. Verified:

/self/i.test('const myself = {}')  → true  ← BLOCKED (false positive)
/self/i.test('itself')             → true  ← BLOCKED (false positive)

Any user function that uses a variable containing "self" will be rejected with a 400. This will break a large proportion of real-world functions. If the intent is to block the Web Worker global self, use \bself\b (with word boundaries), and consider whether blocking the bare identifier is even necessary given Tier 2 runtime sandboxing.

[Software Engineering / Functionality] Bracket notation regex blocks all string array literals

backend/src/services/functions/function.service.ts:375

The third alternative of [\s*['"][a-zA-Z]/i catches any [ followed by a quote and a letter — which is exactly how array literals are written:

const arr = ['hello', 'world'];         // ← BLOCKED
JSON.stringify({ vals: ['a', 'b'] });   // ← BLOCKED
return new Response(body, { headers: ['Content-Type', 'application/json'] });  // ← BLOCKED

Every tested real-world function using string array literals is rejected. The intent (blocking obj['Deno']) requires matching identifier[ not ['. A narrower pattern would be:

/\bDeno\s*\[|\bprocess\s*\[|\bglobalThis\s*\[/

This is separate from the Deno\s*\[|process\s*\[ alternatives already in the regex.

[Security] env: true contradicts the "Secret Shielding" claim

functions/server.ts:168

The Worker is created with env: true, which grants the worker's native Deno runtime unrestricted access to all host environment variables — including JWT_SECRET and any other secrets present in the host process environment. The JS-level sterileEnv polyfill in worker-template.js attempts to shadow Deno.env, but:

1. It only affects the JS layer. Deno native APIs available at the C++ layer remain unrestricted.
2. The Object.defineProperty call uses configurable: true, meaning user code could re-defineProperty to restore the original.
3. The silent catch (e) {} (line 43–45 of worker-template.js) means if the shadow fails silently, real Deno.env is left fully exposed with no error surfaced.

The correct permission for "Secret Shielding" is env: false (no env access) combined with passing only the needed secrets via the secrets message payload — which is already what the existing mockDeno.env.get() mechanism does. The native env permission should be false, not true.

[Security] docker-compose.yml regresses to --allow-all

docker-compose.yml:191

The dev compose was --allow-net --allow-env --allow-read=./functions/worker-template.js and is now --allow-all. This gives the host Deno server process (not the worker) completely unrestricted OS access in dev — filesystem, subprocess execution, all env vars. The whole rationale of this PR is defense-in-depth, but the host process now has broader permissions than before. Dev and prod should diverge no more than necessary, and developers running the dev stack are no longer getting meaningful sandbox signal.

The --allow-all was presumably added to silence a permission error after --unstable-worker-options was introduced, but the correct fix is to identify which specific permission is needed and add only that.

Suggestion

[Software Engineering] Object.defineProperty with configurable: true is weaker than expected
functions/worker-template.js:35

The intent is to lock down Deno.env, but configurable: true allows user code to call Object.defineProperty(globalThis, 'Deno', { value: originalDeno }) and restore the original. Use configurable: false, writable: false to make the shadow permanent. (This is a belt-and-suspenders issue since env: false at the native level would make this moot.)

[Software Engineering] Silent catch on security-critical setup
functions/worker-template.js:43–45

Security-critical polyfill failure should not be silent. If Object.defineProperty throws, the host should receive an error message and terminate the worker, not silently continue with a potentially real Deno.env.

[Software Engineering] Tests are missing coverage for self and constructor patterns
backend/tests/unit/function-security.test.ts

The test file has no test cases for /self/i or /constructor\b/i, both newly added. Add:

it('should block self access', () => {
  expect(() => validateCode('const x = self.postMessage;')).toThrow();
});
it('should block constructor access', () => {
  expect(() => validateCode('obj.constructor.name')).toThrow();
});

[Software Engineering] Tests probe a private method directly
backend/tests/unit/function-security.test.ts:43

(service as any).validateCode(code) bypasses TypeScript's encapsulation. Prefer testing through the public createFunction / updateFunction API with a mocked database, so tests stay valid if the private method is renamed or restructured.

[Functionality] Worker net permission allows backend connections — document the intent
functions/server.ts:169–170

net: ['localhost:7130', 'insforge:7130'] permits workers to initiate outbound connections to the InsForge backend. This appears to be required for the SDK (createClient). It should be documented with a comment so future reviewers understand this is deliberate and not a sandbox leak.

Information

• /import\b/i may be redundant: User code runs inside new Function(...) (CommonJS-style wrapper), where ES module import declarations are a syntax error regardless. Dynamic import() may still work depending on Deno's new Function behavior — if so, the pattern is load-bearing and should be clarified in a comment.
• Comment on bracket regex is misleading: // Block bracket notation property access like obj['Deno'] does not reflect that the actual regex also blocks all string array literals. The comment should match the regex's real behavior once fixed.
• Removal of detailed mock-Deno comments reduces onboarding clarity for contributors unfamiliar with why user code receives a fake Deno. Consider retaining a brief explanation.

Verdict

request_changes — four Critical findings must be addressed before merging. Fixing env: true → env: false, restricting --allow-all in docker-compose.yml, adding word boundaries to /self/i, and narrowing the bracket notation regex are all straightforward changes that don't alter the overall architecture.

LGTM, approved.

Summary

The defense-in-depth model (Tier 1 regex gating + Tier 2 native Deno Worker sandbox) is sound and addresses the original RCE vector well — but a concrete security regression was introduced in the dev Compose file that must be fixed before merge.

Requirements context

No matching spec or plan document was found under /docs/superpowers/specs/ or /docs/superpowers/plans/ for this PR. Requirements assessed against the PR description and referenced issue #1104 alone.

Findings

Critical

Security — docker-compose.yml:191: --allow-read broadened to entire filesystem

The dev Compose command was changed from:

deno run --allow-net --allow-env --allow-read=./functions/worker-template.js --watch functions/server.ts

to:

deno run --allow-net --allow-env --allow-read --unstable-worker-options --watch functions/server.ts

--allow-read without a path argument grants the Deno server process read access to the entire container filesystem — including any mounted secrets, .env files, private keys, or other sensitive files that might exist in the container. The prod file (docker-compose.prod.yml) correctly retains the scoped --allow-read=./functions/worker-template.js.

server.ts only ever reads worker-template.js via Deno.readTextFile (line 27). No other file reads were introduced in this PR. There is no justified reason for the broader permission.

Fix: Keep the scoped flag and append the new flag:

deno run --allow-net --allow-env --allow-read=./functions/worker-template.js --unstable-worker-options --watch functions/server.ts

Suggestion

Security — function.service.ts:381: Error message leaks exact regex patterns to API callers

throw new AppError(
  `Code contains potentially dangerous pattern: ${pattern.toString()}`,
  400,
  ERROR_CODES.INVALID_INPUT
);

The full regex (e.g., /\bDeno\s*\[|\bprocess\s*\[|\bglobalThis\s*\[/i) is returned to the caller in the 400 response. This tells an adversary exactly which patterns to avoid when crafting obfuscated bypasses, reducing the efficacy of Tier 1 and placing more load on Tier 2 to catch what Tier 1 misses.

Fix: Use a generic message: "Code contains a potentially dangerous pattern." Log the matched pattern internally instead.

Security/Functionality — functions/worker-template.js:35-38: Frozen global Deno retains all original APIs

Object.defineProperty(globalThis, 'Deno', {
  value: Object.freeze({ ...originalDeno, env: mockDenoEnv }),
  configurable: false,
  writable: false,
});

Spreading originalDeno means globalThis.Deno.run, globalThis.Deno.spawn, and globalThis.Deno.Command remain present and callable at the JS layer (they will throw at the native layer due to Worker permissions, but they exist). User code that bypasses the Deno parameter injection could still reach these via globalThis.Deno. A minimal mock surface is a cleaner defense:

Object.defineProperty(globalThis, 'Deno', {
  value: Object.freeze({ env: mockDenoEnv }),
  configurable: false,
  writable: false,
});

Software Engineering — function.service.ts:367: /process\b/i has no leading word boundary

The pattern /process\b/i without a leading \b matches identifiers that end with "process" — e.g., subprocess, preprocess. User code like const preprocessData = ... would be rejected with a confusing error.

Fix: / \bprocess\b/i

Software Engineering — Missing test coverage for updateFunction path

backend/tests/unit/function-security.test.ts exercises createFunction exclusively. validateCode is also called from updateFunction (function.service.ts:221), meaning a future regression in the update path would not be caught by these tests.

Fix: Add at least one blocking test that goes through updateFunction (e.g., service.updateFunction('test-function', { code: 'eval(...)' })).

Software Engineering — New regex patterns (import, process, Deno.spawn) have no corresponding tests

The diff adds five new patterns to the blocklist. Only some of them are exercised by the new test suite:

Add tests for the uncovered patterns (process.exit(), import('./x'), require('fs'), Deno.spawn(...)).

Information

functions/worker-template.js:14-17: NODE_ENV: 'production' hardcoded in sterileEnv

sterileEnv sets NODE_ENV: 'production' for every worker invocation, including during local development. Libraries that gate debug output or feature flags on NODE_ENV (e.g., React, some ORMs) will silently behave as if in production. This is probably acceptable given the sandboxing goal but worth documenting or making configurable.

function.service.ts:374: constructor\b/i is broader than the documented intent

The PR intends to block prototype-chain manipulation (obj.constructor.prototype), but the pattern also rejects legitimate type guards like if (x.constructor === Array) or class Foo { constructor() {} }. The latter case is interesting — user functions with ES class syntax are blocked. This trade-off is defensible in a locked-down execution environment but should be noted in developer-facing docs or error messages so users understand why their class definitions are rejected.

Verdict

request_changes — one Critical finding must be addressed: the unjustified broadening of --allow-read in docker-compose.yml is a direct security regression. All other items are non-blocking Suggestions or informational notes.

3 issues found across 6 files

Confidence score: 1/5

• Merge risk is critical because functions/server.ts defines permissions directly on deno instead of deno.permissions, which can cause workers to inherit parent permissions and undermine the intended security boundary.
• backend/src/services/functions/function.service.ts uses /import\b/i, which blocks valid static ES module imports that your provider supports, creating a concrete user-facing regression risk for normal deployments.
• functions/worker-template.js relies on self.close() after security init failure, but synchronous/top-level-await code can still execute, so dynamic imports may still trigger side effects when startup should abort.
• Pay close attention to functions/server.ts, backend/src/services/functions/function.service.ts, functions/worker-template.js - permission isolation, import handling, and abort-on-failure behavior are all high-impact.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="functions/server.ts">

<violation number="1" location="functions/server.ts:166">
P0: Permissions are placed directly on the `deno` object instead of nested under `deno.permissions`. According to the Deno Worker API docs, this structure is not recognized — the worker will silently inherit all parent permissions, completely defeating the sandbox.

Also, `ffi` should be explicitly set to `false` to prevent foreign function interface access.</violation>
</file>

<file name="functions/worker-template.js">

<violation number="1" location="functions/worker-template.js:52">
P2: `self.close()` does not stop synchronous/top-level-await execution. After a security initialization failure, the dynamic imports below will still run (potentially triggering side-effects in imported libraries). Add a `throw` after `self.close()` or wrap the remaining code in a guard to ensure nothing executes after a failed security setup.</violation>
</file>

<file name="backend/src/services/functions/function.service.ts">

<violation number="1" location="backend/src/services/functions/function.service.ts:369">
P1: `/import\b/i` blocks all `import` statements, including legitimate ES module static imports that the deployment provider explicitly supports (see `transformUserCode` in deno-subhosting.provider.ts: "Deno-native format - use as-is, user imports directly"). This will reject valid user functions like `import { createClient } from 'npm:@insforge/sdk';`. Consider narrowing to only dynamic imports (`/import\s*\(/i`) which are the actual sandbox escape risk, while allowing static `import ... from` declarations.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

2 issues found across 4 files (changes from recent commits).

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="functions/server.ts">

<violation number="1" location="functions/server.ts:176">
P2: Missing `sys` and `import` permission restrictions — unspecified permissions inherit from the parent process. Since this PR targets zero-trust sandboxing, explicitly set `sys: false` and `import: false` (or scope `import` to the same whitelist as `net`) to prevent information disclosure via `Deno.hostname()`/`Deno.osRelease()` and uncontrolled dynamic imports.</violation>
</file>

<file name="backend/src/services/functions/function.service.ts">

<violation number="1" location="backend/src/services/functions/function.service.ts:369">
P2: Regex bypass: `import\s*\(` doesn't account for JS comments between `import` and `(`. An attacker can write `import /* */ ('data:...')` to evade this check. Consider a more robust pattern like `/import\s*(?:\/\*[\s\S]*?\*\/\s*|(?:\/\/[^\n]*\n\s*))*\(/i` or, more practically, match `import` followed by `(` anywhere later on the same logical expression: `/\bimport\b[^;]*\(/i`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.}

Summary

Good defense-in-depth concept, but two blocking issues: the net whitelist silently breaks all external API calls from serverless functions (a core use case), and the /constructor\b/i regex false-positives on standard class syntax.

Requirements context

No spec or plan under /docs/superpowers/specs/ or /docs/superpowers/plans/ matches this PR's scope. Review is assessed against the PR description and the linked issue #1104 (RCE remediation via two-tier sandbox isolation) alone.

Findings

Critical

Security / Functionality — Net whitelist breaks all external outbound calls (functions/server.ts:168-177)

net: ['localhost:7130', 'insforge:7130'],

Before this PR the worker inherited --allow-net (unrestricted) from the Deno host process. Now it is locked to two internal hosts. Any user function that makes an outbound HTTP call — to Stripe, OpenAI, SendGrid, an external database, or any third-party API — will fail at runtime with a Deno permission error. This is a silent, undocumented regression of a core serverless feature. The PR description makes no mention of this restriction.

If the intent is to allow only SDK calls to the InsForge backend, the change is correct but must be explicitly documented as a breaking change and validated with the product team. If external calls should remain possible, the net value should be true (allow all, matching the prior behavior) or a configurable allow-list.

Software Engineering / Functionality — /constructor\b/i regex rejects valid class syntax (backend/src/services/functions/function.service.ts:374)

/constructor\b/i,

This pattern matches any occurrence of the literal word constructor, including the constructor() method of a JS/TS class:

class MyHelper {
  constructor(private db: Database) {}  // ← REJECTED
}

Users writing class-based code will receive a cryptic "dangerous pattern" error with no indication of which pattern triggered it (the generic message hides it). The test suite has no regression case for this, making the false positive invisible until users hit it. The guard should target proto-chain abuse specifically, e.g. /__proto__|\.constructor\s*\[/i rather than the bare word.

Suggestion

Security — Tier 1 regex has exploitable gaps; Tier 2 should be documented as the real enforcement boundary (function.service.ts:364-376)

The regex suite can be bypassed with computed property access on non-whitelisted objects:

// 'constructor' literal never appears; neither does 'Function' or 'eval'
(0)['constr' + 'uctor']['constr' + 'uctor']('return this')()

The bracket-notation check (/\bDeno\s*\[|\bprocess\s*\[|\bglobalThis\s*\[/i) only covers three specific global names; any other path to Function is open. The Tier 2 native run: false sandbox is the actual enforcement boundary and would block the resulting RCE attempt. The PR description implies Tier 1 is a meaningful barrier — but it is best understood as a convenience filter (rejects obvious bad actors at the API layer). A comment stating this clearly would prevent future engineers from trusting Tier 1 as a security guarantee.

Functionality — Falsy secret values silently return undefined (functions/worker-template.js:80)

get: (key) => secrets[key] || undefined,

If a secret value is an empty string or "0", || short-circuits to undefined, making the secret appear unset. Prefer secrets[key] ?? undefined (or just secrets[key]).

Software Engineering — originalDeno is dead code in a security-critical path (functions/worker-template.js:34)

const originalDeno = globalThis.Deno;

This captures a reference to the real Deno handle but is never used. While it is scoped to the try block and inaccessible to user code, dead code in a security-critical section invites confusion. Remove it.

Information

sterileEnv declares DEBUG: undefined; has('DEBUG') returns true (functions/worker-template.js:14-17)

The has method (key in sterileEnv) returns true for DEBUG because the key exists even though its value is undefined. Some debug-style libraries enable themselves when has returns true. Omitting DEBUG from the object entirely is cleaner.

sys and hrtime Deno permissions not explicitly denied (functions/server.ts:164-178)

The Deno Worker permissions spec supports sys and hrtime. Explicitly setting sys: false, hrtime: false would make the lockdown manifest complete and self-documenting.

Logger emits internal regex string in structured log (function.service.ts:380-382)

pattern: pattern.toString(),

This is fine for server-side logs but worth noting in case log data is ever forwarded to a user-facing surface — it would expose internal filter logic.

Verdict

request_changes — two Critical findings must be resolved before merge:

1. The net whitelist needs a decision (allow external calls or document the breaking change explicitly).
2. The /constructor\b/i pattern must be narrowed to avoid rejecting valid class syntax.

2 issues found across 4 files (changes from recent commits).

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="functions/server.ts">

<violation number="1" location="functions/server.ts:172">
P0: Setting `net: true` grants the sandboxed worker unrestricted outbound network access, which is a security regression from the previous allowlist (`['localhost:7130', 'insforge:7130']`). Since all decrypted secrets are passed into the worker via `postMessage`, any user-supplied function code can exfiltrate them to an external server. Restore a network allowlist or use a proxy/gateway pattern to control egress.</violation>
</file>

<file name="backend/src/services/functions/function.service.ts">

<violation number="1" location="backend/src/services/functions/function.service.ts:374">
P2: The constructor regex change introduces a known Tier 1 bypass via the Function constructor chain escape (`"".constructor.constructor("malicious")()`) which the old regex would have blocked. Consider also matching `.constructor(` or `.constructor.` to cover direct constructor chaining without breaking class declarations.</violation>
</file>

_{Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic}

Code Review — security: implement two-tier sandbox isolation for serverless functions

Summary: The defense-in-depth architecture (Tier 1 regex filter + Tier 2 native Deno Worker permissions) is sound, the critical enforcement boundary is correctly placed at the runtime layer, and the implementation is substantially more robust than the prior blacklist. No blocking issues found.

Requirements context

No matching spec or plan was found under /docs/superpowers/specs/ or /docs/superpowers/plans/ for this security fix. Review was conducted against the PR description, the linked issue (#1104), and the existing codebase. The three-stage manual audit evidence in the PR body is helpful context.

Findings

Critical

(none)

Suggestion

[Software Engineering] No CI coverage for Tier 2 enforcement — backend/tests/unit/function-security.test.ts

All 16 new tests exercise Tier 1 only (the regex validation in FunctionService.validateCode). There are no integration tests that actually spawn a Worker with run: false and assert that injected-malicious code is rejected by the Deno runtime. The PR's manual audit is compelling, but without a CI-verified integration test the Tier 2 guarantee is invisible to the test suite. A future change that accidentally removes the deno: { permissions: { run: false } } config block would sail through CI undetected.

[Functionality] import: false may conflict with the worker's own dynamic imports — functions/worker-template.js:59-61 / functions/server.ts:166-181

The Worker is granted import: false, yet worker-template.js immediately relies on top-level await import('npm:@insforge/sdk') and await import('https://deno.land/std@0.224.0/encoding/base64.ts'). Whether Deno's unstable import permission applies to module-scoped top-level awaits or only to dynamic imports inside handler code is not clearly documented. If it applies to both, the worker silently fails to initialize on every request. This case works in your manual test, which suggests it may not affect initial module evaluation — but please add an explicit integration test or a note confirming the Deno version-specific behavior so this doesn't regress on a Deno upgrade.

[Security] Tier 1 bypass via variable aliasing — backend/src/services/functions/function.service.ts:364-376

The pattern suite blocks Deno["run"] (via /\bDeno\s*\[/i) but does not block a bare Deno reference. The payload const d = Deno; d["run"]({cmd:["whoami"]}) passes all Tier 1 patterns: Deno alone has no pattern, and d["run"] doesn't match /\bDeno\s*\[/. Tier 2 catches this at the Deno syscall level, so there is no actual RCE risk — but since Tier 1 is advertised as an API-layer "gatekeeper", closing this gap is straightforward: adding /\bDeno\b/i (i.e., any standalone Deno reference) would block this class of bypass. Note: a Deno word-boundary pattern is unlikely to produce false positives in serverless function code.

[Security] Unrestricted net: true enables SSRF to internal infrastructure — functions/server.ts:173

Workers have full outbound network access. This allows user functions to reach cloud metadata endpoints (169.254.169.254, fd00:ec2::254, etc.), internal databases, and other services on the host network. For a shared-tenant deployment this is a meaningful risk. Consider: (a) restricting net to an allowlist of approved domains/CIDR ranges, or (b) explicitly documenting this as a known accepted trade-off with compensating controls (host firewall, VPC egress rules). Either way it should be a deliberate, visible decision rather than an implicit one.

Information

--unstable-worker-options tracks an unstable Deno API — docker-compose.yml:200, docker-compose.prod.yml:174

The granular Worker permission API is still unstable. Pin a specific Deno version in the image tag (if not already done outside this diff) and re-validate permission semantics on each Deno upgrade.

Comment inaccuracy in validateCode — backend/src/services/functions/function.service.ts:379

The Tier 1 block comment says permissions: false but the actual worker uses a granular object (e.g., net: true). Worth correcting to avoid misleading future maintainers.

Test singleton isolation — backend/tests/unit/function-security.test.ts:47-50

FunctionService.getInstance() in beforeEach returns the same singleton across all tests; vi.clearAllMocks() resets mock call counts but does not re-instantiate the service. This is fine now because the service holds no per-request state, but it's a fragile pattern. Consider a test helper that explicitly resets the singleton, or document why it's safe.

Verdict

approved (informational — a human must give the explicit GitHub approval). No Critical findings. The Tier 2 native enforcement is the real security boundary and it is correctly implemented. The Suggestions above — particularly the import: false + dynamic-import interaction and the missing Tier 2 integration test — are worth addressing before this reaches a high-traffic production deployment.