Skip to content

Conversation

@NiloCK
Copy link
Collaborator

@NiloCK NiloCK commented Jul 3, 2025

Toward #791

PR

  • ads a running packages/express backend to the cli studio mode, enabling attachment processing hooks
  • adds database pack command to express server, and exposes it as a server_request
  • connects studio-ui to this server command

Pending testing & documentation updates, this probably achieves basic goals of #791.

  • package express app in cli
  • add expressManager helper
  • add working docs re: express integration w/ studio
  • run & shutdown of studio express server
  • add pack_course endpoint to express server

@NiloCK NiloCK changed the title packed cli dbg Add express server to studio mode, add Flush method Jul 3, 2025
const fs = fsExtra.default || fsExtra;

try {
if (await fs.pathExists(outputPath)) {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix

AI 5 months ago

To fix the issue, we need to validate and sanitize the data.outputPath before using it to construct file paths. The best approach is to ensure that the resolved path is contained within a safe root directory. This involves:

  1. Normalizing the path using path.resolve to remove any .. segments.
  2. Using fs.realpathSync to resolve symbolic links.
  3. Verifying that the normalized path starts with the intended root directory.

Additionally, we can use an allowlist or sanitize the filename if only simple filenames are expected.

Changes are required in packages/express/src/client-requests/pack-requests.ts to validate data.outputPath before constructing outputPath.


Suggested changeset 1
packages/express/src/client-requests/pack-requests.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/express/src/client-requests/pack-requests.ts b/packages/express/src/client-requests/pack-requests.ts
--- a/packages/express/src/client-requests/pack-requests.ts
+++ b/packages/express/src/client-requests/pack-requests.ts
@@ -39,14 +39,22 @@
     if (data.outputPath) {
-      // If output path is provided, check if it's absolute or relative
+      // Validate and sanitize the provided output path
       const pathModule = await import('path');
+      const fsModule = await import('fs');
       const path = pathModule.default || pathModule;
+      const fs = fsModule.default || fsModule;
       
-      if (path.isAbsolute(data.outputPath)) {
-        // Use absolute path as-is
-        outputPath = data.outputPath;
-      } else {
-        // Relative path - combine with project path in studio mode
-        const projectPath = process.env.PROJECT_PATH || process.cwd();
-        outputPath = path.join(projectPath, data.outputPath);
+      const safeRoot = ENV.NODE_ENV === 'studio' ?
+        '/tmp/skuilder-studio-output' :
+        process.cwd();
+      
+      // Resolve the path relative to the safe root
+      const resolvedPath = path.resolve(safeRoot, data.outputPath);
+      
+      // Ensure the resolved path is within the safe root directory
+      const realPath = fs.realpathSync(resolvedPath);
+      if (!realPath.startsWith(safeRoot)) {
+        throw new Error(`Invalid output path: ${data.outputPath}`);
       }
+      
+      outputPath = realPath;
     } else {
EOF
@@ -39,14 +39,22 @@
if (data.outputPath) {
// If output path is provided, check if it's absolute or relative
// Validate and sanitize the provided output path
const pathModule = await import('path');
const fsModule = await import('fs');
const path = pathModule.default || pathModule;
const fs = fsModule.default || fsModule;

if (path.isAbsolute(data.outputPath)) {
// Use absolute path as-is
outputPath = data.outputPath;
} else {
// Relative path - combine with project path in studio mode
const projectPath = process.env.PROJECT_PATH || process.cwd();
outputPath = path.join(projectPath, data.outputPath);
const safeRoot = ENV.NODE_ENV === 'studio' ?
'/tmp/skuilder-studio-output' :
process.cwd();

// Resolve the path relative to the safe root
const resolvedPath = path.resolve(safeRoot, data.outputPath);

// Ensure the resolved path is within the safe root directory
const realPath = fs.realpathSync(resolvedPath);
if (!realPath.startsWith(safeRoot)) {
throw new Error(`Invalid output path: ${data.outputPath}`);
}

outputPath = realPath;
} else {
Copilot is powered by AI and may make mistakes. Always verify output.
try {
if (await fs.pathExists(outputPath)) {
logger.info(`Removing existing directory: ${outputPath}`);
await fs.remove(outputPath);

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix

AI 5 months ago

To fix the issue, we need to validate and sanitize the outputPath before using it. The best approach is to normalize the path using path.resolve and ensure it remains within a designated root directory. This prevents directory traversal attacks and ensures the path is safe to use. If the path is invalid or outside the root directory, the operation should be aborted with an appropriate error response.

Changes required:

  1. Introduce a designated root directory for file operations (e.g., /tmp/skuilder-output).
  2. Normalize outputPath using path.resolve.
  3. Check that the normalized path starts with the root directory.
  4. Reject paths that fail validation with an appropriate error message.
Suggested changeset 1
packages/express/src/client-requests/pack-requests.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/express/src/client-requests/pack-requests.ts b/packages/express/src/client-requests/pack-requests.ts
--- a/packages/express/src/client-requests/pack-requests.ts
+++ b/packages/express/src/client-requests/pack-requests.ts
@@ -38,20 +38,20 @@
     
+    const pathModule = await import('path');
+    const path = pathModule.default || pathModule;
+    const ROOT_DIR = '/tmp/skuilder-output'; // Define a safe root directory
+    
     if (data.outputPath) {
-      // If output path is provided, check if it's absolute or relative
-      const pathModule = await import('path');
-      const path = pathModule.default || pathModule;
+      // Normalize the provided path
+      const resolvedPath = path.resolve(ROOT_DIR, data.outputPath);
       
-      if (path.isAbsolute(data.outputPath)) {
-        // Use absolute path as-is
-        outputPath = data.outputPath;
-      } else {
-        // Relative path - combine with project path in studio mode
-        const projectPath = process.env.PROJECT_PATH || process.cwd();
-        outputPath = path.join(projectPath, data.outputPath);
+      // Ensure the resolved path is within the root directory
+      if (!resolvedPath.startsWith(ROOT_DIR)) {
+        throw new Error(`Invalid outputPath: ${data.outputPath}. Path must be within ${ROOT_DIR}.`);
       }
+      outputPath = resolvedPath;
     } else {
       // No output path provided - use default
-      outputPath = ENV.NODE_ENV === 'studio' ?
-        '/tmp/skuilder-studio-output' :
-        process.cwd();
+      outputPath = path.resolve(ROOT_DIR, ENV.NODE_ENV === 'studio' ?
+        'skuilder-studio-output' :
+        'default-output');
     }
EOF
@@ -38,20 +38,20 @@

const pathModule = await import('path');
const path = pathModule.default || pathModule;
const ROOT_DIR = '/tmp/skuilder-output'; // Define a safe root directory

if (data.outputPath) {
// If output path is provided, check if it's absolute or relative
const pathModule = await import('path');
const path = pathModule.default || pathModule;
// Normalize the provided path
const resolvedPath = path.resolve(ROOT_DIR, data.outputPath);

if (path.isAbsolute(data.outputPath)) {
// Use absolute path as-is
outputPath = data.outputPath;
} else {
// Relative path - combine with project path in studio mode
const projectPath = process.env.PROJECT_PATH || process.cwd();
outputPath = path.join(projectPath, data.outputPath);
// Ensure the resolved path is within the root directory
if (!resolvedPath.startsWith(ROOT_DIR)) {
throw new Error(`Invalid outputPath: ${data.outputPath}. Path must be within ${ROOT_DIR}.`);
}
outputPath = resolvedPath;
} else {
// No output path provided - use default
outputPath = ENV.NODE_ENV === 'studio' ?
'/tmp/skuilder-studio-output' :
process.cwd();
outputPath = path.resolve(ROOT_DIR, ENV.NODE_ENV === 'studio' ?
'skuilder-studio-output' :
'default-output');
}
Copilot is powered by AI and may make mistakes. Always verify output.
@NiloCK NiloCK merged commit adc0e10 into master Jul 4, 2025
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants