security: address remaining code scanning alerts (#253)

This commit is contained in:
Vasanth T
2026-04-03 20:16:53 +05:30
committed by GitHub
parent c1e5e363cd
commit 931ee96f5a
10 changed files with 87 additions and 36 deletions

View File

@@ -120,19 +120,18 @@ function applyFastFlags(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
return env return env
} }
function printSummary(profile: ProviderProfile, env: NodeJS.ProcessEnv): void { function printSummary(profile: ProviderProfile): void {
console.log(`Launching profile: ${profile}`) console.log(`Launching profile: ${profile}`)
if (profile === 'gemini') { if (profile === 'gemini') {
console.log(`GEMINI_MODEL=${env.GEMINI_MODEL}`) console.log('Using configured Gemini provider settings.')
} else if (profile === 'codex') { } else if (profile === 'codex') {
console.log(`OPENAI_BASE_URL=${env.OPENAI_BASE_URL}`) console.log('Using configured Codex/OpenAI-compatible provider settings.')
console.log(`OPENAI_MODEL=${env.OPENAI_MODEL}`)
} else if (profile === 'atomic-chat') { } else if (profile === 'atomic-chat') {
console.log(`OPENAI_BASE_URL=${env.OPENAI_BASE_URL}`) console.log('Using configured Atomic Chat provider settings.')
console.log(`OPENAI_MODEL=${env.OPENAI_MODEL}`) } else if (profile === 'ollama') {
console.log('Using configured Ollama provider settings.')
} else { } else {
console.log(`OPENAI_BASE_URL=${env.OPENAI_BASE_URL}`) console.log('Using configured OpenAI-compatible provider settings.')
console.log(`OPENAI_MODEL=${env.OPENAI_MODEL}`)
} }
} }
@@ -227,7 +226,7 @@ async function main(): Promise<void> {
} }
} }
printSummary(profile, env) printSummary(profile)
const doctorCode = await runProcess('bun', ['run', 'scripts/system-check.ts'], env) const doctorCode = await runProcess('bun', ['run', 'scripts/system-check.ts'], env)
if (doctorCode !== 0) { if (doctorCode !== 0) {

View File

@@ -325,7 +325,7 @@ export async function mcpGetHandler(name: string): Promise<void> {
if (server.oauth.clientId) { if (server.oauth.clientId) {
parts.push('oauth client configured'); parts.push('oauth client configured');
} }
if (server.oauth.callbackPort) parts.push(`callback_port ${server.oauth.callbackPort}`); if (server.oauth.callbackPort) parts.push('callback port configured');
// biome-ignore lint/suspicious/noConsole:: intentional console output // biome-ignore lint/suspicious/noConsole:: intentional console output
console.log(` OAuth: ${parts.join(', ')}`); console.log(` OAuth: ${parts.join(', ')}`);
} }
@@ -347,7 +347,7 @@ export async function mcpGetHandler(name: string): Promise<void> {
if (server.oauth.clientId) { if (server.oauth.clientId) {
parts.push('oauth client configured'); parts.push('oauth client configured');
} }
if (server.oauth.callbackPort) parts.push(`callback_port ${server.oauth.callbackPort}`); if (server.oauth.callbackPort) parts.push('callback port configured');
// biome-ignore lint/suspicious/noConsole:: intentional console output // biome-ignore lint/suspicious/noConsole:: intentional console output
console.log(` OAuth: ${parts.join(', ')}`); console.log(` OAuth: ${parts.join(', ')}`);
} }

View File

@@ -282,17 +282,16 @@ function InstallGitHubApp(props: {
return; return;
} }
const repoWarnings: Warning[] = []; const repoWarnings: Warning[] = [];
if (repoName_1.includes('github.com')) {
const slug = extractGitHubRepoSlug(repoName_1); const slug = extractGitHubRepoSlug(repoName_1);
if (!slug) { const isUrlLike = /^[a-z][a-z0-9+.-]*:\/\//i.test(repoName_1) || repoName_1.startsWith('www.');
if (slug) {
repoName_1 = slug;
} else if (isUrlLike) {
repoWarnings.push({ repoWarnings.push({
title: 'Invalid GitHub URL format', title: 'Invalid GitHub URL format',
message: 'The repository URL format appears to be invalid.', message: 'The repository URL format appears to be invalid.',
instructions: ['Use format: owner/repo or https://github.com/owner/repo', 'Example: anthropics/claude-cli'] instructions: ['Use format: owner/repo or https://github.com/owner/repo', 'Example: anthropics/claude-cli']
}); });
} else {
repoName_1 = slug;
}
} }
if (!repoName_1.includes('/')) { if (!repoName_1.includes('/')) {
repoWarnings.push({ repoWarnings.push({

View File

@@ -33,4 +33,16 @@ test('rejects malformed or non-GitHub URLs', () => {
assert.equal(extractGitHubRepoSlug('https://gitlab.com/Gitlawb/openclaude'), null) assert.equal(extractGitHubRepoSlug('https://gitlab.com/Gitlawb/openclaude'), null)
assert.equal(extractGitHubRepoSlug('https://github.com/Gitlawb'), null) assert.equal(extractGitHubRepoSlug('https://github.com/Gitlawb'), null)
assert.equal(extractGitHubRepoSlug('not actually github.com/Gitlawb/openclaude'), null) assert.equal(extractGitHubRepoSlug('not actually github.com/Gitlawb/openclaude'), null)
assert.equal(
extractGitHubRepoSlug('https://evil.example/?next=github.com/Gitlawb/openclaude'),
null,
)
assert.equal(
extractGitHubRepoSlug('https://github.com.evil.example/Gitlawb/openclaude'),
null,
)
assert.equal(
extractGitHubRepoSlug('https://example.com/github.com/Gitlawb/openclaude'),
null,
)
}) })

View File

@@ -1,12 +1,24 @@
export function extractGitHubRepoSlug(value: string): string | null { export function extractGitHubRepoSlug(value: string): string | null {
const trimmed = value.trim() const trimmed = value.trim()
if (/^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed) && !trimmed.includes('github.com')) { const slugMatch = trimmed.match(
return null /^(?<owner>[^/:\s]+)\/(?<repo>[^/\s]+?)(?:\.git)?\/?$/i,
)
if (slugMatch?.groups?.owner && slugMatch.groups.repo) {
return `${slugMatch.groups.owner}/${slugMatch.groups.repo}`.replace(
/\.git$/i,
'',
)
} }
if (!trimmed.includes('github.com')) { const shorthandUrlMatch = trimmed.match(
return trimmed /^(?:https?:\/\/)?(?:www\.)?github\.com\/(?<owner>[^/:\s]+)\/(?<repo>[^/\s]+?)(?:\.git)?\/?$/i,
)
if (shorthandUrlMatch?.groups?.owner && shorthandUrlMatch.groups.repo) {
return `${shorthandUrlMatch.groups.owner}/${shorthandUrlMatch.groups.repo}`.replace(
/\.git$/i,
'',
)
} }
const sshMatch = trimmed.match( const sshMatch = trimmed.match(
@@ -16,6 +28,10 @@ export function extractGitHubRepoSlug(value: string): string | null {
return `${sshMatch.groups.owner}/${sshMatch.groups.repo}` return `${sshMatch.groups.owner}/${sshMatch.groups.repo}`
} }
if (/^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed)) {
return null
}
try { try {
const parsed = new URL(trimmed) const parsed = new URL(trimmed)
const hostname = parsed.hostname.toLowerCase() const hostname = parsed.hostname.toLowerCase()

View File

@@ -0,0 +1,27 @@
import assert from 'node:assert/strict'
import test from 'node:test'
import {
generateCodeChallenge,
generateCodeVerifier,
generateState,
} from './crypto.ts'
test('generateCodeChallenge returns the RFC 7636 S256 challenge', async () => {
const challenge = await generateCodeChallenge(
'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk',
)
assert.equal(challenge, 'E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM')
})
test('generateCodeVerifier returns a URL-safe random string', () => {
const verifier = generateCodeVerifier()
assert.match(verifier, /^[A-Za-z0-9_-]+$/)
assert.ok(verifier.length >= 43)
})
test('generateState returns a URL-safe random string', () => {
const state = generateState()
assert.match(state, /^[A-Za-z0-9_-]+$/)
assert.ok(state.length >= 43)
})

View File

@@ -1,4 +1,4 @@
import { createHash, randomBytes } from 'crypto' import { randomBytes, webcrypto } from 'crypto'
function base64URLEncode(buffer: Buffer): string { function base64URLEncode(buffer: Buffer): string {
return buffer return buffer
@@ -12,10 +12,10 @@ export function generateCodeVerifier(): string {
return base64URLEncode(randomBytes(32)) return base64URLEncode(randomBytes(32))
} }
export function generateCodeChallenge(verifier: string): string { export async function generateCodeChallenge(verifier: string): Promise<string> {
const hash = createHash('sha256') const encoded = new TextEncoder().encode(verifier)
hash.update(verifier) const digest = await webcrypto.subtle.digest('SHA-256', encoded)
return base64URLEncode(hash.digest()) return base64URLEncode(Buffer.from(digest))
} }
export function generateState(): string { export function generateState(): string {

View File

@@ -52,7 +52,7 @@ export class OAuthService {
this.port = await this.authCodeListener.start() this.port = await this.authCodeListener.start()
// Generate PKCE values and state // Generate PKCE values and state
const codeChallenge = crypto.generateCodeChallenge(this.codeVerifier) const codeChallenge = await crypto.generateCodeChallenge(this.codeVerifier)
const state = crypto.generateState() const state = crypto.generateState()
// Build auth URLs for both automatic and manual flows // Build auth URLs for both automatic and manual flows

View File

@@ -50,7 +50,6 @@ type ExecFileWithCwdOptions = {
maxBuffer?: number maxBuffer?: number
cwd?: string cwd?: string
env?: NodeJS.ProcessEnv env?: NodeJS.ProcessEnv
shell?: boolean | string | undefined
stdin?: 'ignore' | 'inherit' | 'pipe' stdin?: 'ignore' | 'inherit' | 'pipe'
input?: string input?: string
} }
@@ -96,7 +95,6 @@ export function execFileNoThrowWithCwd(
cwd: finalCwd, cwd: finalCwd,
env: finalEnv, env: finalEnv,
maxBuffer, maxBuffer,
shell,
stdin: finalStdin, stdin: finalStdin,
input: finalInput, input: finalInput,
}: ExecFileWithCwdOptions = { }: ExecFileWithCwdOptions = {
@@ -113,7 +111,7 @@ export function execFileNoThrowWithCwd(
timeout: finalTimeout, timeout: finalTimeout,
cwd: finalCwd, cwd: finalCwd,
env: finalEnv, env: finalEnv,
shell, shell: false,
stdin: finalStdin, stdin: finalStdin,
input: finalInput, input: finalInput,
reject: false, // Don't throw on non-zero exit codes reject: false, // Don't throw on non-zero exit codes

View File

@@ -159,7 +159,7 @@ export function logError(error: unknown): void {
const err = toError(error) const err = toError(error)
if (feature('HARD_FAIL') && isHardFailMode()) { if (feature('HARD_FAIL') && isHardFailMode()) {
// biome-ignore lint/suspicious/noConsole:: intentional crash output // biome-ignore lint/suspicious/noConsole:: intentional crash output
console.error('[HARD FAIL] logError called:', err.name || 'Error') console.error('[HARD FAIL] logError called')
// eslint-disable-next-line custom-rules/no-process-exit // eslint-disable-next-line custom-rules/no-process-exit
process.exit(1) process.exit(1)
} }