Skip to content

Commit 39120b4

Browse files
federiconerifedericoneri
authored
Fix open CodeQL alerts and harden logging/workflows (#170)
1 parent 7b0f3cb commit 39120b4

14 files changed

Lines changed: 132 additions & 40 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ on:
66
pull_request:
77
branches: [main]
88

9+
permissions:
10+
contents: read
11+
912
jobs:
1013
test:
1114
runs-on: ubuntu-latest

‎.github/workflows/publish.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ on:
44
push:
55
tags: ['v*']
66

7+
permissions:
8+
contents: read
9+
710
jobs:
811
test:
912
runs-on: ubuntu-latest

‎src/agent/resolve-config.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export async function resolveAgentEnv(
4646

4747
if (!provider) {
4848
throw new Error(
49-
'No AI provider configured. Run `wiggum init` or set ANTHROPIC_API_KEY, OPENAI_API_KEY, or OPENROUTER_API_KEY.',
49+
'No AI provider configured. Run `wiggum init` or set a supported provider API key.',
5050
);
5151
}
5252

‎src/ai/conversation/url-fetcher.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,36 @@ describe('isUrl', () => {
6464
expect(isUrl('not a url')).toBe(false);
6565
});
6666
});
67+
68+
describe('fetchContent HTML sanitization', () => {
69+
it('removes script/style blocks and preserves encoded angle brackets', async () => {
70+
mockIsGitHubIssueUrl.mockReturnValue(null);
71+
72+
const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue({
73+
ok: true,
74+
status: 200,
75+
statusText: 'OK',
76+
headers: { get: () => 'text/html; charset=utf-8' },
77+
text: async () => `
78+
<html>
79+
<head><style>.x { color: red; }</style></head>
80+
<body>
81+
<script>alert('xss')</script>
82+
Hello&nbsp;&lt;script&gt;safe&lt;/script&gt; &amp; &quot;ok&quot;
83+
</body>
84+
</html>
85+
`,
86+
} as unknown as Response);
87+
88+
const result = await fetchContent('https://example.com/page', '/tmp');
89+
90+
fetchSpy.mockRestore();
91+
92+
expect(result.error).toBeUndefined();
93+
expect(result.content).toContain('Hello');
94+
expect(result.content).not.toContain("alert('xss')");
95+
expect(result.content).toContain('&lt;script&gt;safe&lt;/script&gt;');
96+
expect(result.content).toContain('&amp;');
97+
expect(result.content).toContain('"ok"');
98+
});
99+
});

‎src/ai/conversation/url-fetcher.ts‎

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,30 +37,71 @@ export function isUrl(input: string): boolean {
3737
* Simple extraction that removes scripts, styles, and HTML tags
3838
*/
3939
function extractTextFromHtml(html: string): string {
40-
// Remove script and style tags with their content
41-
let text = html
42-
.replace(/<script[^>]*>[\s\S]*?<\/script>/gi, '')
43-
.replace(/<style[^>]*>[\s\S]*?<\/style>/gi, '')
44-
.replace(/<noscript[^>]*>[\s\S]*?<\/noscript>/gi, '');
40+
// Remove script, style, and noscript blocks before generic tag stripping.
41+
let text = stripElementBlocks(html, 'script');
42+
text = stripElementBlocks(text, 'style');
43+
text = stripElementBlocks(text, 'noscript');
4544

4645
// Remove HTML tags but keep content
4746
text = text.replace(/<[^>]+>/g, ' ');
4847

49-
// Decode common HTML entities
50-
text = text
51-
.replace(/&nbsp;/g, ' ')
52-
.replace(/&amp;/g, '&')
53-
.replace(/&lt;/g, '<')
54-
.replace(/&gt;/g, '>')
55-
.replace(/&quot;/g, '"')
56-
.replace(/&#39;/g, "'");
48+
// Decode only safe presentation entities; keep angle brackets encoded.
49+
text = decodeSafeHtmlEntities(text);
5750

5851
// Clean up whitespace
5952
text = text.replace(/\s+/g, ' ').trim();
6053

6154
return text;
6255
}
6356

57+
/**
58+
* Remove full HTML element blocks (open tag + content + closing tag) using
59+
* deterministic string scanning instead of regex.
60+
*/
61+
function stripElementBlocks(input: string, tagName: string): string {
62+
let output = input;
63+
const openToken = `<${tagName}`;
64+
const closeToken = `</${tagName}`;
65+
66+
while (true) {
67+
const lower = output.toLowerCase();
68+
const openStart = lower.indexOf(openToken);
69+
if (openStart === -1) {
70+
break;
71+
}
72+
73+
const openEnd = lower.indexOf('>', openStart + openToken.length);
74+
if (openEnd === -1) {
75+
output = output.slice(0, openStart);
76+
break;
77+
}
78+
79+
const closeStart = lower.indexOf(closeToken, openEnd + 1);
80+
if (closeStart === -1) {
81+
output = output.slice(0, openStart);
82+
break;
83+
}
84+
85+
const closeEnd = lower.indexOf('>', closeStart + closeToken.length);
86+
if (closeEnd === -1) {
87+
output = output.slice(0, openStart);
88+
break;
89+
}
90+
91+
output = output.slice(0, openStart) + output.slice(closeEnd + 1);
92+
}
93+
94+
return output;
95+
}
96+
97+
/** Decode non-structural entities only (quotes/spaces), preserving `<`/`>`/`&`. */
98+
function decodeSafeHtmlEntities(input: string): string {
99+
return input
100+
.replace(/&nbsp;/gi, ' ')
101+
.replace(/&quot;/gi, '"')
102+
.replace(/&#39;/g, "'");
103+
}
104+
64105
/**
65106
* Fetch content from a URL
66107
*/

‎src/ai/enhancer.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,11 +264,10 @@ export class AIEnhancer {
264264
async enhance(scanResult: ScanResult): Promise<EnhancedScanResult> {
265265
// Check if API key is available
266266
if (!this.isAvailable()) {
267-
const envVar = this.getRequiredEnvVar();
268267
return {
269268
...scanResult,
270269
aiEnhanced: false,
271-
aiError: `API key not found. Set ${envVar} to enable AI enhancement.`,
270+
aiError: 'API key not found. Configure credentials to enable AI enhancement.',
272271
};
273272
}
274273

‎src/ai/providers.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,15 @@ export function hasApiKey(provider: AIProvider): boolean {
137137
*/
138138
function getApiKey(provider: AIProvider): string {
139139
const envVar = API_KEY_ENV_VARS[provider];
140-
const apiKey = process.env[envVar];
140+
const credential = process.env[envVar];
141141

142-
if (!apiKey) {
142+
if (!credential) {
143143
throw new Error(
144-
`API key not found. Set ${envVar} environment variable to use ${provider} provider.`
144+
`API key not found for provider: ${provider}.`
145145
);
146146
}
147147

148-
return apiKey;
148+
return credential;
149149
}
150150

151151
/**

‎src/commands/agent.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ vi.mock('../utils/logger.js', () => ({
7575
}));
7676

7777
import { agentCommand } from './agent.js';
78+
import { logger } from '../utils/logger.js';
7879

7980
describe('agentCommand', () => {
8081
let mockExit: ReturnType<typeof vi.spyOn>;
@@ -127,7 +128,7 @@ describe('agentCommand', () => {
127128

128129
await expect(agentCommand()).rejects.toThrow('process.exit(1)');
129130

130-
expect(consoleErrorSpy).toHaveBeenCalledWith(
131+
expect(logger.error).toHaveBeenCalledWith(
131132
expect.stringContaining('No AI provider configured'),
132133
);
133134
expect(mockCreateAgentOrchestrator).not.toHaveBeenCalled();
@@ -138,7 +139,7 @@ describe('agentCommand', () => {
138139

139140
await expect(agentCommand()).rejects.toThrow('process.exit(1)');
140141

141-
expect(consoleErrorSpy).toHaveBeenCalledWith(
142+
expect(logger.error).toHaveBeenCalledWith(
142143
expect.stringContaining('No GitHub remote detected'),
143144
);
144145
expect(mockCreateAgentOrchestrator).not.toHaveBeenCalled();

‎src/commands/agent.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export async function agentCommand(options: AgentOptions = {}): Promise<void> {
5858
try {
5959
env = await resolveAgentEnv(projectRoot, { model: options.model });
6060
} catch (err) {
61-
console.error(`Error: ${err instanceof Error ? err.message : String(err)}`);
61+
logger.error(`Error: ${err instanceof Error ? err.message : String(err)}`);
6262
process.exit(1);
6363
}
6464

‎src/commands/config.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -215,18 +215,18 @@ export async function handleConfigCommand(
215215
}
216216

217217
const rawService = args[1]?.toLowerCase() ?? '';
218-
const apiKey = args[2];
218+
const value = args[2];
219219
const loopCliSetting = normalizeLoopCliSetting(rawService);
220220

221221
if (loopCliSetting) {
222-
if (!isLoopCliValue(apiKey)) {
223-
logger.error(`Invalid ${loopCliSetting} value: '${apiKey}'. Allowed values: ${LOOP_CLI_VALUES.join(', ')}`);
222+
if (!isLoopCliValue(value)) {
223+
logger.error(`Invalid ${loopCliSetting} value: '${value}'. Allowed values: ${LOOP_CLI_VALUES.join(', ')}`);
224224
return state;
225225
}
226226

227227
try {
228-
await saveLoopCliToConfig(state.projectRoot, loopCliSetting, apiKey);
229-
logger.success(`${loopCliSetting} saved to ralph.config.cjs (${apiKey})`);
228+
await saveLoopCliToConfig(state.projectRoot, loopCliSetting, value);
229+
logger.success(`${loopCliSetting} saved to ralph.config.cjs (${value})`);
230230
console.log('');
231231
} catch (error) {
232232
logger.error(`Failed to save ${loopCliSetting}: ${error instanceof Error ? error.message : String(error)}`);
@@ -254,17 +254,17 @@ export async function handleConfigCommand(
254254
const { envVar } = CONFIGURABLE_SERVICES[service];
255255

256256
// Validate API key format (basic check)
257-
if (!apiKey || apiKey.length < 10) {
257+
if (!value || value.length < 10) {
258258
logger.error('Invalid API key. Key appears too short.');
259259
return state;
260260
}
261261

262262
try {
263263
// Save to .env.local
264-
saveKeyToEnvLocal(state.projectRoot, envVar, apiKey);
264+
saveKeyToEnvLocal(state.projectRoot, envVar, value);
265265

266266
// Also set in current process environment
267-
process.env[envVar] = apiKey;
267+
process.env[envVar] = value;
268268

269269
logger.success(`${envVar} saved to .ralph/.env.local`);
270270
console.log(pc.dim('Restart Wiggum to apply changes to tool availability.'));

0 commit comments

Comments
 (0)