Skip to content

CodeQL 2: fix(security): close remaining CodeQL security alerts #190

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

Open
rlorenzo wants to merge 1 commit into
base: main
Choose a base branch
from
+31 −41
Open

CodeQL 2: fix(security): close remaining CodeQL security alerts #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion scripts/lib/lint-staged-common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ function sanitizeFilePath(filePath, baseDir, allowedExtensions, maxFileSizeMB =

// On Windows, lint-staged may pass paths like C:/path/to/file
// Convert these to proper Windows format first
if (IS_WINDOWS && /^[A-Za-z]:[\\\\/]/.test(filePath)) {
if (IS_WINDOWS && /^[A-Za-z]:[\\/]/.test(filePath)) {
normalizedPath = filePath.replaceAll("/", path.sep)
}

Expand Down
62 changes: 26 additions & 36 deletions scripts/lint-any.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,24 +304,32 @@ function runOxfmtCheck(files, fix) {
* @param {string[]} files - Array of file paths
* @param {boolean} fix - Whether to pass --fix
* @param {boolean} clearCache - Whether to pass --clear-cache
* @returns {{ scriptArgs: string[], tempFile: string | null }}
* @returns {{ scriptArgs: string[], cleanup: (() => void) | null }}
*/
function buildScriptArgs(files, fix, clearCache) {
const scriptArgs = []
let tempFile = null
let cleanup = null

const totalLength = files.reduce((sum, f) => sum + f.length + 1, 0)
if (totalLength > MAX_ARG_LENGTH) {
const RADIX = 36
const SUFFIX_LENGTH = 8
tempFile = path.join(
os.tmpdir(),
`lint-files-${Date.now()}-${Math.random()
.toString(RADIX)
.slice(2, 2 + SUFFIX_LENGTH)}.txt`,
)
fs.writeFileSync(tempFile, files.join("\n"), "utf8")
scriptArgs.push(`--files-from=${tempFile}`)
// Use mkdtempSync (private mode-0700 dir with cryptographic suffix) so the
// inner file cannot be predicted/preempted by another local process.
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "lint-files-"))
const tempFile = path.join(tempDir, "files.txt")
cleanup = () => {
try {
fs.rmSync(tempDir, { recursive: true, force: true })
Comment thread
rlorenzo marked this conversation as resolved.
} catch {
/* Best-effort cleanup */
}
}
try {
fs.writeFileSync(tempFile, files.join("\n"), "utf8")
scriptArgs.push(`--files-from=${tempFile}`)
} catch (error) {
cleanup()
throw error
}
} else {
scriptArgs.push(...files)
}
Expand All @@ -333,7 +341,7 @@ function buildScriptArgs(files, fix, clearCache) {
scriptArgs.push("--clear-cache")
}

return { scriptArgs, tempFile }
return { scriptArgs, cleanup }
}

/**
Expand All @@ -352,21 +360,15 @@ function runLinter(script, files, description, fix, clearCache) {
console.log(`\n🔍 ${description} (${files.length} files)`)

const scriptPath = path.join(__dirname, script)
const { scriptArgs, tempFile } = buildScriptArgs(files, fix, clearCache)
const { scriptArgs, cleanup } = buildScriptArgs(files, fix, clearCache)

const result = spawnSync("node", [scriptPath, ...scriptArgs], {
stdio: "inherit",
cwd: projectRoot,
env,
})

if (tempFile) {
try {
fs.unlinkSync(tempFile)
} catch {
/* Best-effort cleanup */
}
}
cleanup?.()

if (result.error) {
console.error(`❌ Failed to run ${script}:`, result.error.message)
Expand All @@ -392,7 +394,7 @@ function runLinterAsync(script, files, description, fix, clearCache) {
console.log(`\n🔍 ${description} (${files.length} files)`)

const scriptPath = path.join(__dirname, script)
const { scriptArgs, tempFile } = buildScriptArgs(files, fix, clearCache)
const { scriptArgs, cleanup } = buildScriptArgs(files, fix, clearCache)

return new Promise((resolve) => {
const child = spawn("node", [scriptPath, ...scriptArgs], {
Expand All @@ -402,13 +404,7 @@ function runLinterAsync(script, files, description, fix, clearCache) {
})

child.on("close", (code, signal) => {
if (tempFile) {
try {
fs.unlinkSync(tempFile)
} catch {
/* Best-effort cleanup */
}
}
cleanup?.()
if (signal) {
console.error(`❌ ${script} terminated by signal ${signal}`)
resolve(1)
Expand All @@ -417,13 +413,7 @@ function runLinterAsync(script, files, description, fix, clearCache) {
resolve(code ?? 1)
})
child.on("error", (err) => {
if (tempFile) {
try {
fs.unlinkSync(tempFile)
} catch {
/* Best-effort cleanup */
}
}
cleanup?.()
console.error(`❌ Failed to run ${script}:`, err.message)
resolve(1)
})
Expand Down
6 changes: 3 additions & 3 deletions web/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Amazon;

Check warning on line 1 in web/Program.cs

View workflow job for this annotation

GitHub Actions / Backend Tests 

'<Main>$' has a cyclomatic complexity of '41'. Rewrite or refactor the code to decrease its complexity below '26'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1502)
using Amazon.Extensions.NETCore.Setup;
using Amazon.Runtime.CredentialManagement;
using Joonasw.AspNetCore.SecurityHeaders;
Expand Down Expand Up @@ -30,7 +30,7 @@

// Load .env.local for local development only (multiple-instance support)
// Avoid loading in production - guard by ASPNETCORE_ENVIRONMENT.
var envPath = Path.Combine(Directory.GetCurrentDirectory(), "../.env.local");
var envPath = Path.Join(Directory.GetCurrentDirectory(), "../.env.local");

Check warning on line 33 in web/Program.cs

View workflow job for this annotation

GitHub Actions / Backend Tests 

'Program' has a maintainability index of '2'. Rewrite or refactor the code to increase its maintainability index (MI) above '9'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1505)
var aspNetEnv = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT");
if (string.Equals(aspNetEnv, "Development", StringComparison.OrdinalIgnoreCase)
&& File.Exists(envPath))
Expand Down Expand Up @@ -476,7 +476,7 @@
{
DefaultFileNames = new List<string> { "index.html" },
FileProvider = new PhysicalFileProvider(
Path.Combine(builder.Environment.ContentRootPath, "wwwroot/vue")),
Path.Join(builder.Environment.ContentRootPath, "wwwroot/vue")),
RequestPath = "/vue",
RedirectToAppendTrailingSlash = true
});
Expand All @@ -487,7 +487,7 @@
app.UseStaticFiles(new StaticFileOptions
{
FileProvider = new PhysicalFileProvider(
Path.Combine(builder.Environment.ContentRootPath, "wwwroot/vue")),
Path.Join(builder.Environment.ContentRootPath, "wwwroot/vue")),
RequestPath = "/2/vue"
});

Expand Down
2 changes: 1 addition & 1 deletion web/ViteProxyHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ public static async Task HandleProxyError(HttpContext context, Exception ex, ILo
}
}

var physicalPath = Path.Combine(context.RequestServices.GetRequiredService<IWebHostEnvironment>().WebRootPath,
var physicalPath = Path.Join(context.RequestServices.GetRequiredService<IWebHostEnvironment>().WebRootPath,
staticPath.TrimStart('/').Replace('/', Path.DirectorySeparatorChar));

// Prevent directory traversal: ensure the resolved physical path is within WebRootPath
Expand Down
Loading