Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions builder/source/cache.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,28 @@
import * as Zod from 'zod'
import * as Fs from 'node:fs'
import * as Process from 'node:process'
import * as Path from 'node:path'
import { FetchAdShieldDomains } from './references/index.js'

const CachePath = (Process.env.INIT_CWD ? Process.env.INIT_CWD : Process.cwd()) + '/.buildcache'
const CacheDomainsPath = CachePath + '/domains.json'
const ProjectRoot = Path.resolve(Process.cwd())
const EnvInitCwd = Process.env.INIT_CWD && Path.isAbsolute(Process.env.INIT_CWD)
? Path.resolve(Process.env.INIT_CWD)
: null
const BaseCacheDir = (EnvInitCwd && (EnvInitCwd === ProjectRoot || EnvInitCwd.startsWith(ProjectRoot + Path.sep)))
? EnvInitCwd

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix

AI about 22 hours ago

To fix this, we should ensure that any environment-derived path is safely normalized and verified to be within the intended root directory before it is used. That means resolving EnvInitCwd and ProjectRoot to absolute, normalized paths and then ensuring that EnvInitCwd is actually contained within ProjectRoot using a robust check, rather than a simple string startsWith on potentially unnormalized values.

The best minimal fix here is:

  • Normalize ProjectRoot and any candidate EnvInitCwd using Path.resolve.
  • Introduce a small helper function (in this file) isSubPath(parent, child) that:
    • Resolves both arguments.
    • Compares them in a path-separator-aware way, e.g. child === parent or child.startsWith(parent + Path.sep).
  • Use this helper to decide whether to trust EnvInitCwd; otherwise fall back to ProjectRoot.

This keeps behavior the same for normal, valid values of INIT_CWD, but eliminates cases where a crafted path that normalizes outside ProjectRoot or relies on path quirks could be accepted. All required imports (Path, Process) already exist, so we only need to add the helper and adjust the BaseCacheDir computation within builder/source/cache.ts.

Suggested changeset 1
builder/source/cache.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/builder/source/cache.ts b/builder/source/cache.ts
--- a/builder/source/cache.ts
+++ b/builder/source/cache.ts
@@ -5,10 +5,17 @@
 import { FetchAdShieldDomains } from './references/index.js'
 
 const ProjectRoot = Path.resolve(Process.cwd())
+
+function isSubPath(parent: string, child: string): boolean {
+  const parentResolved = Path.resolve(parent)
+  const childResolved = Path.resolve(child)
+  return childResolved === parentResolved || childResolved.startsWith(parentResolved + Path.sep)
+}
+
 const EnvInitCwd = Process.env.INIT_CWD && Path.isAbsolute(Process.env.INIT_CWD)
   ? Path.resolve(Process.env.INIT_CWD)
   : null
-const BaseCacheDir = (EnvInitCwd && (EnvInitCwd === ProjectRoot || EnvInitCwd.startsWith(ProjectRoot + Path.sep)))
+const BaseCacheDir = (EnvInitCwd && isSubPath(ProjectRoot, EnvInitCwd))
   ? EnvInitCwd
   : ProjectRoot
 const CachePath = Path.join(BaseCacheDir, '.buildcache')
EOF
@@ -5,10 +5,17 @@
import { FetchAdShieldDomains } from './references/index.js'

const ProjectRoot = Path.resolve(Process.cwd())

function isSubPath(parent: string, child: string): boolean {
const parentResolved = Path.resolve(parent)
const childResolved = Path.resolve(child)
return childResolved === parentResolved || childResolved.startsWith(parentResolved + Path.sep)
}

const EnvInitCwd = Process.env.INIT_CWD && Path.isAbsolute(Process.env.INIT_CWD)
? Path.resolve(Process.env.INIT_CWD)
: null
const BaseCacheDir = (EnvInitCwd && (EnvInitCwd === ProjectRoot || EnvInitCwd.startsWith(ProjectRoot + Path.sep)))
const BaseCacheDir = (EnvInitCwd && isSubPath(ProjectRoot, EnvInitCwd))
? EnvInitCwd
: ProjectRoot
const CachePath = Path.join(BaseCacheDir, '.buildcache')
Copilot is powered by AI and may make mistakes. Always verify output.
: ProjectRoot
const CachePath = Path.join(BaseCacheDir, '.buildcache')
const CacheDomainsPath = Path.join(CachePath, 'domains.json')

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix

AI about 22 hours ago

In general, to fix uncontrolled path usage derived from environment variables, we should (1) choose a trusted base directory (e.g., the current working directory or a configured application root), (2) resolve any untrusted value against that base using path.resolve, and (3) ensure the resulting path is within the intended directory tree (or fall back to the base if validation fails). For a simple cache directory we can avoid depending on untrusted input entirely, or only use it if it stays within our project root.

For this specific code, the simplest and safest fix without changing observable functionality is:

  • Keep using Process.cwd() as the ultimate trusted base.
  • When INIT_CWD is absolute, resolve it and verify it is inside (or equal to) Process.cwd(). If so, we can use it; otherwise ignore it and fall back to Process.cwd().
  • Alternatively, if we want to eliminate the taint entirely, we can remove use of INIT_CWD and always base the cache on Process.cwd().

To minimize behavioral change while still satisfying CodeQL and hardening security, we’ll validate INIT_CWD against Process.cwd():

  1. Compute const ProjectRoot = Path.resolve(Process.cwd()).
  2. If Process.env.INIT_CWD is set and absolute, compute const initCwd = Path.resolve(Process.env.INIT_CWD).
  3. Check that initCwd starts with ProjectRoot plus a path separator (or is exactly equal). If not, ignore it.
  4. Set BaseCacheDir to the validated initCwd when valid, or to ProjectRoot otherwise.
  5. Build CachePath from BaseCacheDir as before.

All changes are in builder/source/cache.ts at the top where BaseCacheDir and CachePath are defined; no new imports are required because Path is already imported.

Suggested changeset 1
builder/source/cache.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/builder/source/cache.ts b/builder/source/cache.ts
--- a/builder/source/cache.ts
+++ b/builder/source/cache.ts
@@ -4,10 +4,14 @@
 import * as Path from 'node:path'
 import { FetchAdShieldDomains } from './references/index.js'
 
-const BaseCacheDir = (Process.env.INIT_CWD && Path.isAbsolute(Process.env.INIT_CWD))
-  ? Process.env.INIT_CWD
-  : Process.cwd()
-const CachePath = Path.join(Path.resolve(BaseCacheDir), '.buildcache')
+const ProjectRoot = Path.resolve(Process.cwd())
+const EnvInitCwd = Process.env.INIT_CWD && Path.isAbsolute(Process.env.INIT_CWD)
+  ? Path.resolve(Process.env.INIT_CWD)
+  : null
+const BaseCacheDir = (EnvInitCwd && (EnvInitCwd === ProjectRoot || EnvInitCwd.startsWith(ProjectRoot + Path.sep)))
+  ? EnvInitCwd
+  : ProjectRoot
+const CachePath = Path.join(BaseCacheDir, '.buildcache')
 const CacheDomainsPath = Path.join(CachePath, 'domains.json')
 
 export function CreateCache(Domains: Set<string>) {
EOF
@@ -4,10 +4,14 @@
import * as Path from 'node:path'
import { FetchAdShieldDomains } from './references/index.js'

const BaseCacheDir = (Process.env.INIT_CWD && Path.isAbsolute(Process.env.INIT_CWD))
? Process.env.INIT_CWD
: Process.cwd()
const CachePath = Path.join(Path.resolve(BaseCacheDir), '.buildcache')
const ProjectRoot = Path.resolve(Process.cwd())
const EnvInitCwd = Process.env.INIT_CWD && Path.isAbsolute(Process.env.INIT_CWD)
? Path.resolve(Process.env.INIT_CWD)
: null
const BaseCacheDir = (EnvInitCwd && (EnvInitCwd === ProjectRoot || EnvInitCwd.startsWith(ProjectRoot + Path.sep)))
? EnvInitCwd
: ProjectRoot
const CachePath = Path.join(BaseCacheDir, '.buildcache')
const CacheDomainsPath = Path.join(CachePath, 'domains.json')

export function CreateCache(Domains: Set<string>) {
Copilot is powered by AI and may make mistakes. Always verify output.
@piquark6046 piquark6046 committed this autofix suggestion about 22 hours ago.

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix

AI about 22 hours ago

In general, to fix uncontrolled path use when combining a root directory with user-controlled components, always normalize the candidate path (using path.resolve and ideally fs.realpathSync), and then verify that the normalized path is within an expected root (via a robust prefix check) before using it. Never rely on naive string concatenation or startsWith checks on unnormalized paths.

For this specific code, the intent is to allow INIT_CWD to point somewhere within the project directory tree and, if it does, to use that as the base for .buildcache; otherwise, default to ProjectRoot. The best way to fix this without changing behavior is:

  1. Normalize Process.env.INIT_CWD with Path.resolve before any containment check.
  2. Optionally call Fs.realpathSync (wrapped in a try/catch) to resolve symlinks; if resolution fails, treat it as invalid and ignore INIT_CWD.
  3. Perform the containment check on the normalized path: ensure it is equal to ProjectRoot or is strictly inside it. For the “inside” case, a robust check is: normalizedPath === ProjectRoot || (normalizedPath.startsWith(ProjectRoot + Path.sep)).
  4. Only if the check passes, use that normalized value as BaseCacheDir; otherwise, set BaseCacheDir to ProjectRoot.

We can implement this by slightly restructuring the EnvInitCwd / BaseCacheDir initialization block at the top of builder/source/cache.ts. No new external dependencies are needed; we can reuse node:fs and node:path, which are already imported. The rest of the logic using CachePath and CacheDomainsPath can remain unchanged.

Suggested changeset 1
builder/source/cache.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/builder/source/cache.ts b/builder/source/cache.ts
--- a/builder/source/cache.ts
+++ b/builder/source/cache.ts
@@ -5,8 +5,16 @@
 import { FetchAdShieldDomains } from './references/index.js'
 
 const ProjectRoot = Path.resolve(Process.cwd())
-const EnvInitCwd = Process.env.INIT_CWD && Path.isAbsolute(Process.env.INIT_CWD)
-  ? Path.resolve(Process.env.INIT_CWD)
+const EnvInitCwdRaw = Process.env.INIT_CWD
+const EnvInitCwd = EnvInitCwdRaw && Path.isAbsolute(EnvInitCwdRaw)
+  ? (() => {
+    const resolved = Path.resolve(EnvInitCwdRaw)
+    try {
+      return Fs.realpathSync(resolved)
+    } catch {
+      return null
+    }
+  })()
   : null
 const BaseCacheDir = (EnvInitCwd && (EnvInitCwd === ProjectRoot || EnvInitCwd.startsWith(ProjectRoot + Path.sep)))
   ? EnvInitCwd
EOF
@@ -5,8 +5,16 @@
import { FetchAdShieldDomains } from './references/index.js'

const ProjectRoot = Path.resolve(Process.cwd())
const EnvInitCwd = Process.env.INIT_CWD && Path.isAbsolute(Process.env.INIT_CWD)
? Path.resolve(Process.env.INIT_CWD)
const EnvInitCwdRaw = Process.env.INIT_CWD
const EnvInitCwd = EnvInitCwdRaw && Path.isAbsolute(EnvInitCwdRaw)
? (() => {
const resolved = Path.resolve(EnvInitCwdRaw)
try {
return Fs.realpathSync(resolved)
} catch {
return null
}
})()
: null
const BaseCacheDir = (EnvInitCwd && (EnvInitCwd === ProjectRoot || EnvInitCwd.startsWith(ProjectRoot + Path.sep)))
? EnvInitCwd
Copilot is powered by AI and may make mistakes. Always verify output.

export function CreateCache(Domains: Set<string>) {
if (!Fs.existsSync(CachePath)) {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Fs.mkdirSync(CachePath)
} else if (!Fs.statSync(CachePath).isDirectory()) {
throw new Error('.buildcache exists and is not a directory!')
}

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
if (Fs.existsSync(CacheDomainsPath)) {
throw new Error('Cache already exists!')
}

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Fs.writeFileSync(CacheDomainsPath, JSON.stringify([...Domains], null, 2), { encoding: 'utf-8' })
}

Expand All @@ -28,7 +36,7 @@
try {
new URLPattern(`https://${Value}/`)
return true
} catch {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
return false
}
})).parseAsync(DomainsArray)
Expand Down
Loading