Skip to content

Commit 44cd088

Browse files
Copilotsualeh
andauthored
Add path sanitization and reliability documentation updates
Agent-Logs-Url: https://github.com/schemacrawler/SchemaCrawler/sessions/d783923d-31e1-48fd-944b-ab9fcda0d376 Co-authored-by: sualeh <36505153+sualeh@users.noreply.github.com>
1 parent dc0d2a6 commit 44cd088

2 files changed

Lines changed: 43 additions & 6 deletions

File tree

RELIABILITY_CHANGES.md

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,30 @@
44

55
1. **Hardened credential file handling**
66
- Updated:
7-
- `/home/runner/work/SchemaCrawler/SchemaCrawler/schemacrawler-commandline/src/main/java/schemacrawler/tools/commandline/command/UserOptions.java`
8-
- `/home/runner/work/SchemaCrawler/SchemaCrawler/schemacrawler-commandline/src/main/java/schemacrawler/tools/commandline/command/PasswordOptions.java`
7+
- `schemacrawler-commandline/src/main/java/schemacrawler/tools/commandline/command/UserOptions.java`
8+
- `schemacrawler-commandline/src/main/java/schemacrawler/tools/commandline/command/PasswordOptions.java`
99
- Changes:
1010
- Normalize the provided path before use.
1111
- Validate that the path is a **readable regular file**.
1212
- Read only the **first line** from the file using a buffered reader (instead of loading all lines).
1313

14-
2. **Added targeted regression tests**
14+
2. **Sanitized generated completion-script file name input**
1515
- Updated:
16-
- `/home/runner/work/SchemaCrawler/SchemaCrawler/schemacrawler-commandline/src/test/java/schemacrawler/test/commandline/command/UserParserTest.java`
17-
- `/home/runner/work/SchemaCrawler/SchemaCrawler/schemacrawler-commandline/src/test/java/schemacrawler/test/commandline/command/PasswordParserTest.java`
16+
- `schemacrawler-commandline/src/main/java/schemacrawler/tools/commandline/utility/GenerateCliSupport.java`
17+
- Changes:
18+
- Reject empty, absolute, traversal-like, and multi-segment values for `--completion-script`.
19+
- Ensure the resolved output path stays within the requested output directory.
20+
21+
3. **Added targeted regression tests**
22+
- Updated:
23+
- `schemacrawler-commandline/src/test/java/schemacrawler/test/commandline/command/UserParserTest.java`
24+
- `schemacrawler-commandline/src/test/java/schemacrawler/test/commandline/command/PasswordParserTest.java`
1825
- New tests verify that using a directory path for `--user:file` or `--password:file` fails gracefully.
1926

2027
## Why these changes are recommended
2128

2229
- **Safer file-system processing:** Explicitly rejecting non-file and unreadable paths avoids ambiguous IO behavior and produces clearer failures.
2330
- **Improved resilience:** Reading a single credential line avoids unnecessary memory use on unexpectedly large files.
2431
- **Predictable behavior:** Path normalization and upfront validation reduce edge-case handling surprises.
32+
- **Input sanitization:** Restricting completion script file names prevents unintended path resolution.
2533
- **Maintained compatibility:** Existing behavior (using the first line as the credential) is preserved.

schemacrawler-commandline/src/main/java/schemacrawler/tools/commandline/utility/GenerateCliSupport.java

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package schemacrawler.tools.commandline.utility;
1010

1111
import java.io.IOException;
12+
import java.nio.file.InvalidPathException;
1213
import java.nio.file.Files;
1314
import java.nio.file.Path;
1415
import java.util.Arrays;
@@ -59,7 +60,7 @@ public void run() {
5960
boolean isErrored = false;
6061

6162
outputDir = createOutputDirectory();
62-
final Path completionScript = outputDir.resolve(completionScriptFilename).toAbsolutePath();
63+
final Path completionScript = createCompletionScriptPath();
6364

6465
if (generateAsciiDoc()) {
6566
isErrored = true;
@@ -82,6 +83,34 @@ protected Path createOutputDirectory() {
8283
return outputDir.toAbsolutePath();
8384
}
8485

86+
private Path createCompletionScriptPath() {
87+
final Path outputDirPath = outputDir.toAbsolutePath().normalize();
88+
final String filename =
89+
completionScriptFilename == null ? "" : completionScriptFilename.trim();
90+
final Path scriptFileName;
91+
try {
92+
scriptFileName = Path.of(filename);
93+
} catch (final InvalidPathException e) {
94+
throw new IllegalArgumentException(
95+
"Completion script file name is invalid <%s>".formatted(completionScriptFilename), e);
96+
}
97+
98+
if (filename.isEmpty()
99+
|| scriptFileName.isAbsolute()
100+
|| scriptFileName.getNameCount() != 1
101+
|| ".".equals(scriptFileName.toString())
102+
|| "..".equals(scriptFileName.toString())) {
103+
throw new IllegalArgumentException(
104+
"Completion script file name must be a simple file name");
105+
}
106+
107+
final Path completionScript = outputDirPath.resolve(scriptFileName).normalize();
108+
if (!completionScript.startsWith(outputDirPath)) {
109+
throw new IllegalArgumentException("Completion script path resolves outside output directory");
110+
}
111+
return completionScript;
112+
}
113+
85114
private CommandLine createCommandLine(final Object commands) {
86115
final ShellState state = new ShellState();
87116
final StateFactory stateFactory = new StateFactory(state);

0 commit comments

Comments
 (0)