mirror of
https://github.com/danielmiessler/fabric.git
synced 2026-09-10 07:36:44 -04:00
fix: redact API keys in config responses and eliminate shell injection surfaces
- add `maskAPIKey` to redact all but last 4 chars of API keys (CWE-200) - add `isRedacted` guard to prevent writing masked values back to `.env` - mask all provider API keys in `GET /config` response payload - sanitize note filenames with `basename` and allowlist regex (CWE-78, CWE-22) - replace `exec`/shell commands in obsidian route with native `fs` APIs - remove `escapeShellArg` helper now that shell execution is fully eliminated - add path-confinement double-check ensuring resolved paths stay within target dirs - sanitize note filenames in notes route using `basename` to block path traversal (CWE-22) - return `safeFilename` instead of raw user input in notes POST response
This commit is contained in:
parent
29e1681929
commit
4f9d4875a6
|
|
@ -28,6 +28,23 @@ func NewConfigHandler(r *gin.Engine, db *fsdb.Db) *ConfigHandler {
|
|||
return handler
|
||||
}
|
||||
|
||||
// maskAPIKey redacts all but the last 4 characters of a secret key (CWE-200).
|
||||
// An empty value (key not configured) is returned unchanged so the UI can
|
||||
// distinguish "not set" from "set but redacted".
|
||||
func maskAPIKey(key string) string {
|
||||
const visible = 4
|
||||
if len(key) <= visible {
|
||||
return key
|
||||
}
|
||||
return strings.Repeat("*", len(key)-visible) + key[len(key)-visible:]
|
||||
}
|
||||
|
||||
// isRedacted returns true when a submitted value looks like a masked key
|
||||
// returned by maskAPIKey, signalling that the user did not change the field.
|
||||
func isRedacted(value string) bool {
|
||||
return strings.Contains(value, "*")
|
||||
}
|
||||
|
||||
func (h *ConfigHandler) GetConfig(c *gin.Context) {
|
||||
if h.db == nil {
|
||||
c.JSON(http.StatusNotFound, gin.H{"error": ".env file not found"})
|
||||
|
|
@ -56,17 +73,19 @@ func (h *ConfigHandler) GetConfig(c *gin.Context) {
|
|||
return
|
||||
}
|
||||
|
||||
// API keys are masked to their last 4 characters (CWE-200).
|
||||
// URLs are not secrets and are returned as-is so the UI can display them.
|
||||
config := map[string]string{
|
||||
"openai": os.Getenv("OPENAI_API_KEY"),
|
||||
"anthropic": os.Getenv("ANTHROPIC_API_KEY"),
|
||||
"groq": os.Getenv("GROQ_API_KEY"),
|
||||
"mistral": os.Getenv("MISTRAL_API_KEY"),
|
||||
"gemini": os.Getenv("GEMINI_API_KEY"),
|
||||
"openai": maskAPIKey(os.Getenv("OPENAI_API_KEY")),
|
||||
"anthropic": maskAPIKey(os.Getenv("ANTHROPIC_API_KEY")),
|
||||
"groq": maskAPIKey(os.Getenv("GROQ_API_KEY")),
|
||||
"mistral": maskAPIKey(os.Getenv("MISTRAL_API_KEY")),
|
||||
"gemini": maskAPIKey(os.Getenv("GEMINI_API_KEY")),
|
||||
"ollama": os.Getenv("OLLAMA_URL"),
|
||||
"openrouter": os.Getenv("OPENROUTER_API_KEY"),
|
||||
"silicon": os.Getenv("SILICON_API_KEY"),
|
||||
"deepseek": os.Getenv("DEEPSEEK_API_KEY"),
|
||||
"grokai": os.Getenv("GROKAI_API_KEY"),
|
||||
"openrouter": maskAPIKey(os.Getenv("OPENROUTER_API_KEY")),
|
||||
"silicon": maskAPIKey(os.Getenv("SILICON_API_KEY")),
|
||||
"deepseek": maskAPIKey(os.Getenv("DEEPSEEK_API_KEY")),
|
||||
"grokai": maskAPIKey(os.Getenv("GROKAI_API_KEY")),
|
||||
"lmstudio": os.Getenv("LM_STUDIO_API_BASE_URL"),
|
||||
}
|
||||
|
||||
|
|
@ -114,7 +133,9 @@ func (h *ConfigHandler) UpdateConfig(c *gin.Context) {
|
|||
|
||||
var envContent strings.Builder
|
||||
for key, value := range envVars {
|
||||
if value != "" {
|
||||
// Skip empty values and redacted placeholders returned by GET /config.
|
||||
// Writing a masked value back would corrupt the stored key.
|
||||
if value != "" && !isRedacted(value) {
|
||||
envContent.WriteString(fmt.Sprintf("%s=%s\n", key, value))
|
||||
os.Setenv(key, value)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
import { json } from '@sveltejs/kit';
|
||||
import type { RequestHandler } from './$types';
|
||||
import { writeFile } from 'fs/promises';
|
||||
import { join } from 'path';
|
||||
import { join, resolve, basename } from 'path';
|
||||
import { fileURLToPath } from 'url';
|
||||
import { dirname } from 'path';
|
||||
|
||||
|
|
@ -17,14 +17,24 @@ export const POST: RequestHandler = async ({ request }) => {
|
|||
// Get the absolute path to the inbox directory
|
||||
const __filename = fileURLToPath(import.meta.url);
|
||||
const __dirname = dirname(__filename);
|
||||
// const inboxPath = join(__dirname, '..', 'myfiles', 'inbox', filename);
|
||||
// New version using environment variables:
|
||||
// const inboxPath = join(process.env.DATA_DIR || './web/myfiles', 'inbox', filename);
|
||||
const inboxPath = join(__dirname, '..', '..', '..', 'myfiles', 'inbox', filename);
|
||||
const inboxDir = resolve(__dirname, '..', '..', '..', 'myfiles', 'inbox');
|
||||
|
||||
// Security: use only the basename to strip any path traversal sequences (CWE-22)
|
||||
const safeFilename = basename(filename);
|
||||
if (!safeFilename) {
|
||||
return json({ error: 'Invalid filename' }, { status: 400 });
|
||||
}
|
||||
|
||||
const inboxPath = join(inboxDir, safeFilename);
|
||||
|
||||
// Double-check the resolved path is still within the inbox directory
|
||||
if (!inboxPath.startsWith(inboxDir + '/') && inboxPath !== inboxDir) {
|
||||
return json({ error: 'Invalid filename' }, { status: 400 });
|
||||
}
|
||||
|
||||
await writeFile(inboxPath, content, 'utf-8');
|
||||
|
||||
return json({ success: true, filename });
|
||||
return json({ success: true, filename: safeFilename });
|
||||
} catch (error) {
|
||||
console.error('Server error:', error);
|
||||
return json(
|
||||
|
|
|
|||
|
|
@ -1,9 +1,7 @@
|
|||
import { json } from '@sveltejs/kit';
|
||||
import type { RequestHandler } from './$types';
|
||||
import { exec } from 'child_process';
|
||||
import { promisify } from 'util';
|
||||
|
||||
const execAsync = promisify(exec);
|
||||
import { mkdir, writeFile, stat } from 'fs/promises';
|
||||
import { resolve, basename, join } from 'path';
|
||||
|
||||
interface ObsidianRequest {
|
||||
pattern: string;
|
||||
|
|
@ -11,14 +9,11 @@ interface ObsidianRequest {
|
|||
content: string;
|
||||
}
|
||||
|
||||
function escapeShellArg(arg: string): string {
|
||||
// Replace single quotes with '\'' and wrap in single quotes
|
||||
return `'${arg.replace(/'/g, "'\\''")}'`;
|
||||
}
|
||||
// Allowlist of safe filename characters — prevents command injection (CWE-78)
|
||||
// and path traversal (CWE-22) via the noteName field.
|
||||
const SAFE_NOTE_NAME = /^[a-zA-Z0-9 _.-]+$/;
|
||||
|
||||
export const POST: RequestHandler = async ({ request }) => {
|
||||
let tempFile: string | undefined;
|
||||
|
||||
try {
|
||||
// Parse and validate request
|
||||
const body = await request.json() as ObsidianRequest;
|
||||
|
|
@ -29,45 +24,44 @@ export const POST: RequestHandler = async ({ request }) => {
|
|||
);
|
||||
}
|
||||
|
||||
// Security: strip directory components then validate against an allowlist.
|
||||
// This prevents shell command injection (CWE-78) — double-quoted interpolation
|
||||
// does not block $(...) or backtick substitution in bash — and path traversal
|
||||
// (CWE-22). Shell execution is eliminated entirely in favour of native fs APIs.
|
||||
const safeNoteName = basename(body.noteName);
|
||||
if (!safeNoteName || !SAFE_NOTE_NAME.test(safeNoteName)) {
|
||||
return json({ error: 'Invalid note name' }, { status: 400 });
|
||||
}
|
||||
|
||||
console.log('\n=== Obsidian Request ===');
|
||||
console.log('1. Pattern:', body.pattern);
|
||||
console.log('2. Note name:', body.noteName);
|
||||
console.log('2. Note name:', safeNoteName);
|
||||
console.log('3. Content length:', body.content.length);
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
// Format content with markdown code blocks
|
||||
const formattedContent = `\`\`\`markdown\n${body.content}\n\`\`\``;
|
||||
const escapedFormattedContent = escapeShellArg(formattedContent);
|
||||
|
||||
// Generate file name and path
|
||||
const fileName = `${new Date().toISOString().split('T')[0]}-${body.noteName}.md`;
|
||||
|
||||
const obsidianDir = 'myfiles/Fabric_obsidian';
|
||||
const filePath = `${obsidianDir}/${fileName}`;
|
||||
await execAsync(`mkdir -p "${obsidianDir}"`);
|
||||
const fileName = `${new Date().toISOString().split('T')[0]}-${safeNoteName}.md`;
|
||||
const obsidianDir = resolve('myfiles/Fabric_obsidian');
|
||||
const filePath = join(obsidianDir, fileName);
|
||||
|
||||
// Defense-in-depth: confirm the resolved path is inside obsidianDir (CWE-22)
|
||||
if (!filePath.startsWith(obsidianDir + '/') && filePath !== obsidianDir) {
|
||||
return json({ error: 'Invalid note name' }, { status: 400 });
|
||||
}
|
||||
|
||||
// Use native fs APIs — no shell involved, no injection surface
|
||||
await mkdir(obsidianDir, { recursive: true });
|
||||
console.log('4. Ensured Obsidian directory exists');
|
||||
|
||||
await writeFile(filePath, formattedContent, 'utf-8');
|
||||
console.log('5. Wrote content to final location:', filePath);
|
||||
|
||||
// Create temp file
|
||||
tempFile = `/tmp/fabric-${Date.now()}.txt`;
|
||||
|
||||
// Write formatted content to temp file
|
||||
await execAsync(`echo ${escapedFormattedContent} > "${tempFile}"`);
|
||||
console.log('5. Wrote formatted content to temp file');
|
||||
|
||||
// Copy from temp file to final location (safer than direct write)
|
||||
await execAsync(`cp "${tempFile}" "${filePath}"`);
|
||||
console.log('6. Copied content to final location:', filePath);
|
||||
|
||||
// Verify file was created and has content
|
||||
const { stdout: lsOutput } = await execAsync(`ls -l "${filePath}"`);
|
||||
const { stdout: wcOutput } = await execAsync(`wc -l "${filePath}"`);
|
||||
console.log('7. File verification:', lsOutput);
|
||||
console.log('8. Line count:', wcOutput);
|
||||
// Verify file was created
|
||||
const fileStats = await stat(filePath);
|
||||
const lineCount = formattedContent.split('\n').length;
|
||||
console.log('6. File verification: size =', fileStats.size, 'bytes,', lineCount, 'lines');
|
||||
|
||||
// Return success response with file details
|
||||
return json({
|
||||
|
|
@ -82,7 +76,7 @@ export const POST: RequestHandler = async ({ request }) => {
|
|||
console.error('Type:', error?.constructor?.name);
|
||||
console.error('Message:', error instanceof Error ? error.message : String(error));
|
||||
console.error('Stack:', error instanceof Error ? error.stack : 'No stack trace');
|
||||
|
||||
|
||||
return json(
|
||||
{
|
||||
error: error instanceof Error ? error.message : 'Failed to process request',
|
||||
|
|
@ -90,16 +84,5 @@ export const POST: RequestHandler = async ({ request }) => {
|
|||
},
|
||||
{ status: 500 }
|
||||
);
|
||||
|
||||
} finally {
|
||||
// Clean up temp file if it exists
|
||||
if (tempFile) {
|
||||
try {
|
||||
await execAsync(`rm -f "${tempFile}"`);
|
||||
console.log('9. Cleaned up temp file');
|
||||
} catch (cleanupError) {
|
||||
console.error('Failed to clean up temp file:', cleanupError);
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
};
|
||||
|
|
|
|||
Loading…
Reference in a new issue