Skip to content

Potential fix for code scanning alert no. 10: Uncontrolled data used in path expression #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
piquark6046 wants to merge 2 commits into from
Closed

Potential fix for code scanning alert no. 10: Uncontrolled data used in path expression #59

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae2b306
Potential fix for code scanning alert no. 10: Uncontrolled data used …
piquark6046 Mar 11, 2026
a2b96be
Potential fix for code scanning alert no. 11: Uncontrolled data used …
piquark6046 Mar 11, 2026
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
8 changes: 6 additions & 2 deletions builder/source/cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +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 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')

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
Loading
.

Copilot Autofix

AI about 1 month ago

General approach: Normalize and validate any environment-derived path before using it to construct filesystem paths, and ensure it cannot escape a designated safe root (ProjectRoot). This matches the pattern described in the background: resolve/realpath the candidate path, then check that it is within the intended root folder. If validation fails, fall back to a safe default (ProjectRoot).

Best concrete fix here: update the computation of EnvInitCwd and BaseCacheDir so that:

  1. We only trust Process.env.INIT_CWD if it is:

    • an absolute path,
    • successfully canonicalized with Fs.realpathSync,
    • and strictly contained within the canonical ProjectRoot directory.

  2. We avoid path traversal or symlink tricks by realpath’ing both ProjectRoot and INIT_CWD and then checking directory containment using Path.relative, which is more robust than startsWith.

  3. If any of these conditions fail, we set EnvInitCwd to null and BaseCacheDir falls back to ProjectRoot as before.

Concretely, in builder/source/cache.ts:

  • Introduce a RealProjectRoot computed via Fs.realpathSync(ProjectRoot), with a safe fallback to ProjectRoot if resolution fails.
  • Replace the current inline ternary that sets EnvInitCwd and the subsequent BaseCacheDir check with logic that:
    • Resolves Process.env.INIT_CWD via Fs.realpathSync only if it’s absolute.
    • Uses Path.relative(RealProjectRoot, realInitCwd) and checks that it is not absolute, does not start with '..', and is not equal to '..'.
    • Only if this containment check passes, sets EnvInitCwd to the realpath’d directory; otherwise, leaves it null.
  • Keep everything else (CachePath, CacheDomainsPath, and the cache read/write functions) unchanged.

No new external dependencies are needed; we already import Fs, Path, and Process.

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,12 +5,33 @@
 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)
-  : null
-const BaseCacheDir = (EnvInitCwd && (EnvInitCwd === ProjectRoot || EnvInitCwd.startsWith(ProjectRoot + Path.sep)))
-  ? EnvInitCwd
-  : ProjectRoot
+const RealProjectRoot = (() => {
+  try {
+    return Fs.realpathSync(ProjectRoot)
+  } catch {
+    return ProjectRoot
+  }
+})()
+
+const EnvInitCwd = (() => {
+  const initCwd = Process.env.INIT_CWD
+  if (!initCwd || !Path.isAbsolute(initCwd)) {
+    return null
+  }
+  let realInitCwd: string
+  try {
+    realInitCwd = Fs.realpathSync(initCwd)
+  } catch {
+    return null
+  }
+  const relative = Path.relative(RealProjectRoot, realInitCwd)
+  if (!relative || relative === '.' || (relative && !relative.startsWith('..') && !Path.isAbsolute(relative))) {
+    return realInitCwd
+  }
+  return null
+})()
+
+const BaseCacheDir = EnvInitCwd ?? RealProjectRoot
 const CachePath = Path.join(BaseCacheDir, '.buildcache')
 const CacheDomainsPath = Path.join(CachePath, 'domains.json')
 
EOF
@@ -5,12 +5,33 @@
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)
: null
const BaseCacheDir = (EnvInitCwd && (EnvInitCwd === ProjectRoot || EnvInitCwd.startsWith(ProjectRoot + Path.sep)))
? EnvInitCwd
: ProjectRoot
const RealProjectRoot = (() => {
try {
return Fs.realpathSync(ProjectRoot)
} catch {
return ProjectRoot
}
})()

const EnvInitCwd = (() => {
const initCwd = Process.env.INIT_CWD
if (!initCwd || !Path.isAbsolute(initCwd)) {
return null
}
let realInitCwd: string
try {
realInitCwd = Fs.realpathSync(initCwd)
} catch {
return null
}
const relative = Path.relative(RealProjectRoot, realInitCwd)
if (!relative || relative === '.' || (relative && !relative.startsWith('..') && !Path.isAbsolute(relative))) {
return realInitCwd
}
return null
})()

const BaseCacheDir = EnvInitCwd ?? RealProjectRoot
const CachePath = Path.join(BaseCacheDir, '.buildcache')
const CacheDomainsPath = Path.join(CachePath, 'domains.json')

Copilot is powered by AI and may make mistakes. Always verify output.
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
Loading
.

Copilot Autofix

AI about 1 month 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 1 month ago.

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
Loading
.

Copilot Autofix

AI about 1 month 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)) {
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
Loading
.
if (Fs.existsSync(CacheDomainsPath)) {
throw new Error('Cache already exists!')
}
Fs.writeFileSync(CacheDomainsPath, JSON.stringify([...Domains], null, 2), { encoding: 'utf-8' })

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
Loading
.
}

export async function LoadCache(): Promise<Set<string>> {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
Loading
.
if (!Fs.existsSync(CacheDomainsPath)) {
throw new Error('Cache does not exist!')
}
Expand All @@ -32,7 +36,7 @@
return false
}
})).parseAsync(DomainsArray)
return new Set(DomainsArray)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
Loading
.
}

export async function LoadDomainsFromCache(): Promise<Set<string>> {
Expand Down
Loading