-
Notifications
You must be signed in to change notification settings - Fork 542
Improve code docker #197
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.
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
Closed
Closed
Improve code docker #197
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
8ff4b59
refactor: implement ALLOWED_ROOT_DIRECTORY security and fix path vali…
3a0a2e3
refactor: remove WORKSPACE_DIR, use only ALLOWED_ROOT_DIRECTORY
0bcd522
refactor: remove unused OPENAI_API_KEY and GOOGLE_API_KEY
873429d
Merge branch 'main' of github.com:AutoMaker-Org/automaker
ade8048
fix: enforce ALLOWED_ROOT_DIRECTORY path validation across all routes
f3c9e82
refactor: integrate secure file system operations across services
86d92e6
refactor: streamline ALLOWED_ROOT_DIRECTORY handling and remove legac…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| /** | ||
| * Secure File System Adapter | ||
| * | ||
| * All file I/O operations must go through this adapter to enforce | ||
| * ALLOWED_ROOT_DIRECTORY restrictions at the actual access point, | ||
| * not just at the API layer. This provides defense-in-depth security. | ||
| */ | ||
|
|
||
| import fs from "fs/promises"; | ||
| import path from "path"; | ||
| import { validatePath } from "./security.js"; | ||
|
|
||
| /** | ||
| * Wrapper around fs.access that validates path first | ||
| */ | ||
| export async function access(filePath: string, mode?: number): Promise<void> { | ||
| const validatedPath = validatePath(filePath); | ||
| return fs.access(validatedPath, mode); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.readFile that validates path first | ||
| */ | ||
| export async function readFile( | ||
| filePath: string, | ||
| encoding?: BufferEncoding | ||
| ): Promise<string | Buffer> { | ||
| const validatedPath = validatePath(filePath); | ||
| if (encoding) { | ||
| return fs.readFile(validatedPath, encoding); | ||
| } | ||
| return fs.readFile(validatedPath); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.writeFile that validates path first | ||
| */ | ||
| export async function writeFile( | ||
| filePath: string, | ||
| data: string | Buffer, | ||
| encoding?: BufferEncoding | ||
| ): Promise<void> { | ||
| const validatedPath = validatePath(filePath); | ||
| return fs.writeFile(validatedPath, data, encoding as any); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.mkdir that validates path first | ||
| */ | ||
| export async function mkdir( | ||
| dirPath: string, | ||
| options?: { recursive?: boolean; mode?: number } | ||
| ): Promise<string | undefined> { | ||
| const validatedPath = validatePath(dirPath); | ||
| return fs.mkdir(validatedPath, options); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.readdir that validates path first | ||
| */ | ||
| export async function readdir( | ||
| dirPath: string, | ||
| options?: { withFileTypes?: boolean; encoding?: BufferEncoding } | ||
| ): Promise<string[] | any[]> { | ||
| const validatedPath = validatePath(dirPath); | ||
| return fs.readdir(validatedPath, options as any); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.stat that validates path first | ||
| */ | ||
| export async function stat(filePath: string): Promise<any> { | ||
| const validatedPath = validatePath(filePath); | ||
| return fs.stat(validatedPath); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.rm that validates path first | ||
| */ | ||
| export async function rm( | ||
| filePath: string, | ||
| options?: { recursive?: boolean; force?: boolean } | ||
| ): Promise<void> { | ||
| const validatedPath = validatePath(filePath); | ||
| return fs.rm(validatedPath, options); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.unlink that validates path first | ||
| */ | ||
| export async function unlink(filePath: string): Promise<void> { | ||
| const validatedPath = validatePath(filePath); | ||
| return fs.unlink(validatedPath); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.copyFile that validates both paths first | ||
| */ | ||
| export async function copyFile( | ||
| src: string, | ||
| dest: string, | ||
| mode?: number | ||
| ): Promise<void> { | ||
| const validatedSrc = validatePath(src); | ||
| const validatedDest = validatePath(dest); | ||
| return fs.copyFile(validatedSrc, validatedDest, mode); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.appendFile that validates path first | ||
| */ | ||
| export async function appendFile( | ||
| filePath: string, | ||
| data: string | Buffer, | ||
| encoding?: BufferEncoding | ||
| ): Promise<void> { | ||
| const validatedPath = validatePath(filePath); | ||
| return fs.appendFile(validatedPath, data, encoding as any); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.rename that validates both paths first | ||
| */ | ||
| export async function rename( | ||
| oldPath: string, | ||
| newPath: string | ||
| ): Promise<void> { | ||
| const validatedOldPath = validatePath(oldPath); | ||
| const validatedNewPath = validatePath(newPath); | ||
| return fs.rename(validatedOldPath, validatedNewPath); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around fs.lstat that validates path first | ||
| * Returns file stats without following symbolic links | ||
| */ | ||
| export async function lstat(filePath: string): Promise<any> { | ||
| const validatedPath = validatePath(filePath); | ||
| return fs.lstat(validatedPath); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around path.join that returns resolved path | ||
| * Does NOT validate - use this for path construction, then pass to other operations | ||
| */ | ||
| export function joinPath(...pathSegments: string[]): string { | ||
| return path.join(...pathSegments); | ||
| } | ||
|
|
||
| /** | ||
| * Wrapper around path.resolve that returns resolved path | ||
| * Does NOT validate - use this for path construction, then pass to other operations | ||
| */ | ||
| export function resolvePath(...pathSegments: string[]): string { | ||
| return path.resolve(...pathSegments); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 105
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 2633
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 101
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 5361
Fix the
secureFs.readFiletype signature with function overloads.The
as Buffercast at line 66 is unsafe becausesecureFs.readFilereturnsPromise<string | Buffer>without distinguishing between calls with and without encoding. Updatesecure-fs.tsto use function overloads matching Node'sfs.readFilebehavior:This eliminates the need for type assertion and lets TypeScript infer the correct return type based on whether encoding is provided.
🤖 Prompt for AI Agents