Skip to content

Commit 4cee005

Browse files
Copilotsualeh
andauthored
Use FileInputResource for credential file reading; revert GenerateCliSupport changes
Agent-Logs-Url: https://github.com/schemacrawler/SchemaCrawler/sessions/0c7f56ce-dcc2-489e-80ce-3bf852ca475d Co-authored-by: sualeh <36505153+sualeh@users.noreply.github.com>
1 parent 44cd088 commit 4cee005

4 files changed

Lines changed: 25 additions & 58 deletions

File tree

RELIABILITY_CHANGES.md

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,7 @@
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. **Sanitized generated completion-script file name input**
15-
- Updated:
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**
14+
2. **Added targeted regression tests**
2215
- Updated:
2316
- `schemacrawler-commandline/src/test/java/schemacrawler/test/commandline/command/UserParserTest.java`
2417
- `schemacrawler-commandline/src/test/java/schemacrawler/test/commandline/command/PasswordParserTest.java`
@@ -29,5 +22,5 @@
2922
- **Safer file-system processing:** Explicitly rejecting non-file and unreadable paths avoids ambiguous IO behavior and produces clearer failures.
3023
- **Improved resilience:** Reading a single credential line avoids unnecessary memory use on unexpectedly large files.
3124
- **Predictable behavior:** Path normalization and upfront validation reduce edge-case handling surprises.
32-
- **Input sanitization:** Restricting completion script file names prevents unintended path resolution.
25+
- **Input sanitization via library:** Using `FileInputResource` from `us.fatehi.utility.ioresource` delegates file validation to a shared, well-tested library class.
3326
- **Maintained compatibility:** Existing behavior (using the first line as the credential) is preserved.

schemacrawler-commandline/src/main/java/schemacrawler/tools/commandline/command/PasswordOptions.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.nio.file.Path;
1818
import picocli.CommandLine.Option;
1919
import schemacrawler.schemacrawler.exceptions.IORuntimeException;
20+
import us.fatehi.utility.ioresource.FileInputResource;
2021

2122
public final class PasswordOptions {
2223

@@ -89,20 +90,21 @@ private String getPasswordFromFile() {
8990
return null;
9091
}
9192

92-
final Path normalizedPasswordFile = passwordFile.toAbsolutePath().normalize();
93-
if (!Files.isRegularFile(normalizedPasswordFile) || !Files.isReadable(normalizedPasswordFile)) {
94-
throw new IORuntimeException(
95-
"Password could not be read from file <%s> - path is not a readable regular file"
96-
.formatted(normalizedPasswordFile));
97-
}
98-
9993
try {
100-
try (final BufferedReader reader = Files.newBufferedReader(normalizedPasswordFile, UTF_8)) {
94+
final FileInputResource inputResource = new FileInputResource(passwordFile);
95+
try (final BufferedReader reader = inputResource.openNewInputReader(UTF_8)) {
10196
return reader.readLine();
10297
}
10398
} catch (final IOException e) {
99+
// FileInputResource throws IOException for unreadable, non-regular, AND empty files.
100+
// If the path is a readable regular file the only reason for the exception is that it is
101+
// empty, which means no credential was supplied - return null in that case.
102+
final Path path = passwordFile.toAbsolutePath().normalize();
103+
if (Files.isRegularFile(path) && Files.isReadable(path)) {
104+
return null;
105+
}
104106
throw new IORuntimeException(
105-
"Password could not be read from file <%s>".formatted(normalizedPasswordFile), e);
107+
"Password could not be read from file <%s>".formatted(passwordFile), e);
106108
}
107109
}
108110

schemacrawler-commandline/src/main/java/schemacrawler/tools/commandline/command/UserOptions.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.nio.file.Path;
1818
import picocli.CommandLine.Option;
1919
import schemacrawler.schemacrawler.exceptions.IORuntimeException;
20+
import us.fatehi.utility.ioresource.FileInputResource;
2021

2122
public final class UserOptions {
2223

@@ -89,20 +90,20 @@ private String getUserFromFile() {
8990
return null;
9091
}
9192

92-
final Path normalizedUserFile = userFile.toAbsolutePath().normalize();
93-
if (!Files.isRegularFile(normalizedUserFile) || !Files.isReadable(normalizedUserFile)) {
94-
throw new IORuntimeException(
95-
"User could not be read from file <%s> - path is not a readable regular file"
96-
.formatted(normalizedUserFile));
97-
}
98-
9993
try {
100-
try (final BufferedReader reader = Files.newBufferedReader(normalizedUserFile, UTF_8)) {
94+
final FileInputResource inputResource = new FileInputResource(userFile);
95+
try (final BufferedReader reader = inputResource.openNewInputReader(UTF_8)) {
10196
return reader.readLine();
10297
}
10398
} catch (final IOException e) {
104-
throw new IORuntimeException(
105-
"User could not be read from file <%s>".formatted(normalizedUserFile), e);
99+
// FileInputResource throws IOException for unreadable, non-regular, AND empty files.
100+
// If the path is a readable regular file the only reason for the exception is that it is
101+
// empty, which means no credential was supplied - return null in that case.
102+
final Path path = userFile.toAbsolutePath().normalize();
103+
if (Files.isRegularFile(path) && Files.isReadable(path)) {
104+
return null;
105+
}
106+
throw new IORuntimeException("User could not be read from file <%s>".formatted(userFile), e);
106107
}
107108
}
108109

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

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

1111
import java.io.IOException;
12-
import java.nio.file.InvalidPathException;
1312
import java.nio.file.Files;
1413
import java.nio.file.Path;
1514
import java.util.Arrays;
@@ -60,7 +59,7 @@ public void run() {
6059
boolean isErrored = false;
6160

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

6564
if (generateAsciiDoc()) {
6665
isErrored = true;
@@ -83,34 +82,6 @@ protected Path createOutputDirectory() {
8382
return outputDir.toAbsolutePath();
8483
}
8584

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-
11485
private CommandLine createCommandLine(final Object commands) {
11586
final ShellState state = new ShellState();
11687
final StateFactory stateFactory = new StateFactory(state);

0 commit comments

Comments
 (0)