Skip to content

Fix high-severity security issues found by Snyk Code scan #319

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
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix high-severity security issues found by Snyk Code scan #319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
80b20fb
fix: address high-severity security issues found by Snyk Code scan
devin-ai-integration[bot] Oct 23, 2025
8856099
fix: remove trailing whitespace to pass editorconfig check
devin-ai-integration[bot] Oct 23, 2025
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
12 changes: 12 additions & 0 deletions examples/llama.swiftui/llama.swiftui/UI/DownloadButton.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ struct DownloadButton: View {

do {
if let temporaryURL = temporaryURL {
let tempDir = FileManager.default.temporaryDirectory
guard temporaryURL.path.hasPrefix(tempDir.path) else {
print("Security Error: Temporary file path is outside expected directory")
return
Comment on lines +52 to +54
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UX Issue: Silent Failure

When path validation fails, the code only prints to console with no user feedback. Users will see their download appear to complete but the file won't be saved, leading to confusion.

Current behavior:

guard temporaryURL.path.hasPrefix(tempDir.path) else {
    print("Security Error: Temporary file path is outside expected directory")
    return  // Silent failure - user sees no error
}

Suggested improvement:

guard temporaryURL.path.hasPrefix(tempDir.path) else {
    DispatchQueue.main.async {
        self.status = "download"  // Reset to allow retry
        // Consider adding an @State error message to display in UI
    }
    print("Security Error: Temporary file path is outside expected directory")
    return
}

Consider adding an error state variable to show security errors to users in the UI.

}
Comment on lines +51 to +55
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Issue: Insecure Path Validation

Using hasPrefix for path validation is vulnerable to path traversal attacks. This approach doesn't handle:

  1. Path normalization: Paths with .. components can bypass the check
  2. Symlinks: Symbolic links can point outside the allowed directory
  3. Path canonicalization: Different representations of the same path

Example Attack:

// If tempDir.path = "/tmp"
// An attacker could use: "/tmp/../var/tmp/malicious"
// This passes hasPrefix("/tmp") but resolves outside /tmp

Recommended Fix:

let tempDir = FileManager.default.temporaryDirectory
let resolvedTempURL = temporaryURL.standardizedFileURL.resolvingSymlinksInPath()
let resolvedTempDir = tempDir.standardizedFileURL.resolvingSymlinksInPath()

guard resolvedTempURL.path.hasPrefix(resolvedTempDir.path + "/") || resolvedTempURL.path == resolvedTempDir.path else {
    print("Security Error: Temporary file path is outside expected directory")
    return
}

The same fix should be applied to the destination path validation on lines 57-61.


let docsDir = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0]
guard fileURL.path.hasPrefix(docsDir.path) else {
print("Security Error: Destination path is outside documents directory")
return
}

try FileManager.default.copyItem(at: temporaryURL, to: fileURL)
print("Writing to \(filename) completed")

Expand Down
12 changes: 12 additions & 0 deletions examples/llama.swiftui/llama.swiftui/UI/InputButton.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,18 @@ struct InputButton: View {

do {
if let temporaryURL = temporaryURL {
let tempDir = FileManager.default.temporaryDirectory
guard temporaryURL.path.hasPrefix(tempDir.path) else {
print("Security Error: Temporary file path is outside expected directory")
return
}

let docsDir = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0]
guard fileURL.path.hasPrefix(docsDir.path) else {
print("Security Error: Destination path is outside documents directory")
return
}
Comment on lines +55 to +65
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Issue: Same Path Validation Vulnerability

This code has the same insecure path validation issue as DownloadButton.swift. Using hasPrefix doesn't protect against path traversal attacks with symlinks or .. components.

Please apply the same fix recommended in DownloadButton.swift:

let tempDir = FileManager.default.temporaryDirectory
let resolvedTempURL = temporaryURL.standardizedFileURL.resolvingSymlinksInPath()
let resolvedTempDir = tempDir.standardizedFileURL.resolvingSymlinksInPath()

guard resolvedTempURL.path.hasPrefix(resolvedTempDir.path + "/") || resolvedTempURL.path == resolvedTempDir.path else {
    print("Security Error: Temporary file path is outside expected directory")
    return
}

let docsDir = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask)[0]
let resolvedFileURL = fileURL.standardizedFileURL.resolvingSymlinksInPath()
let resolvedDocsDir = docsDir.standardizedFileURL.resolvingSymlinksInPath()

guard resolvedFileURL.path.hasPrefix(resolvedDocsDir.path + "/") || resolvedFileURL.path == resolvedDocsDir.path else {
    print("Security Error: Destination path is outside documents directory")
    return
}

Also consider adding user-visible error feedback as mentioned in the DownloadButton.swift review.


try FileManager.default.copyItem(at: temporaryURL, to: fileURL)
print("Writing to \(filename) completed")

Expand Down
2 changes: 1 addition & 1 deletion tools/server/webui/src/lib/constants/settings-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const SETTING_CONFIG_DEFAULT: Record<string, string | number | boolean> =
};

export const SETTING_CONFIG_INFO: Record<string, string> = {
apiKey: 'Set the API Key if you are using --api-key option for the server.',
apiKey: 'Configure the API Key for authentication. Never hardcode API keys in source code - use environment variables or secure configuration management instead.',
systemMessage: 'The starting message that defines how model should behave.',
theme:
'Choose the color theme for the interface. You can choose between System (follows your device settings), Light, or Dark.',
Expand Down
Loading