From 8894d24eebaed8184290ef4bc70dfcd91aff0105 Mon Sep 17 00:00:00 2001 From: Hakon Gudbjartsson Date: Fri, 4 Sep 2026 19:25:44 +0000 Subject: [PATCH] fix: default gorRoot to absolute CWD instead of the literal string "./" When -gorroot is omitted, GorSessionFactory.updateCommonRoot() (and DriverBackedFileReader's own matching default) fell back to the literal string "./". Resolving a relative path against "./" is a no-op in this codebase's URI-based path resolution (PathUtils.resolve: resolving anything against a relative base just returns it unchanged), so any code path that needed real anchoring against this default silently got an un-anchored relative path back instead. This surfaces concretely in PGOR's dictionary-folder write path: a direct `pgor | write .gord` (no -gorroot given) fails with a spurious Resource Error on a fabricated version_XXXX.gord name, even though the write's own underlying computation succeeds -- GeneralQueryHandler.getResultsLinkPath's .gord.link bookkeeping resolves the write target against this un-anchored root, produces a bare relative string with no directory information, and something downstream mis-resolves it against the wrong base. Supplying an explicit, absolute -gorroot has always worked around this; the actual bug is that the *default* was never a usable root to begin with. Verified this default is never consulted for access-control decisions: DriverBackedFileReader.validateAccess() is a no-op, and the real enforcement path (DriverBackedSecureFileReader) always sources its project root independently (raw PipeOptions.gorRoot()/ GORMORE_GOR_ROOT/GORMORE_PROJECTS_ROOT, never through updateCommonRoot()) -- so this change only affects path-resolution/ anchoring correctness, not security scoping. Co-Authored-By: Claude Sonnet 5 --- .../gorsat/process/GorSessionFactory.java | 26 ++++++++++++++++--- .../gor/model/DriverBackedFileReader.java | 13 +++++++++- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/gortools/src/main/java/gorsat/process/GorSessionFactory.java b/gortools/src/main/java/gorsat/process/GorSessionFactory.java index 4474b38be..ca671c40f 100644 --- a/gortools/src/main/java/gorsat/process/GorSessionFactory.java +++ b/gortools/src/main/java/gorsat/process/GorSessionFactory.java @@ -25,6 +25,8 @@ import org.gorpipe.gor.session.GenericFactory; import org.gorpipe.gor.session.GorSession; +import java.nio.file.Paths; + public abstract class GorSessionFactory extends GenericFactory { protected static String updateCommonRoot(String commonRootOpt) { @@ -33,10 +35,28 @@ protected static String updateCommonRoot(String commonRootOpt) { } if (commonRootOpt != null) { if (commonRootOpt.trim().length() == 0) { - commonRootOpt = "./"; - } else if (commonRootOpt.length() > 2 && commonRootOpt.charAt(1) == ':' && !commonRootOpt.endsWith("\\")) { // windows path hack + // Default to the process's actual absolute CWD, not the + // literal string "./" -- resolving a relative path + // against "./" is a no-op in this codebase's URI-based + // path resolution (PathUtils.resolve: resolving anything + // against a relative base just returns it unchanged), + // so anything that needed real anchoring against this + // default quietly stayed relative/un-anchored instead + // (e.g. PGOR's dictionary-folder write caching -- see + // the .gord.link mechanism in GeneralQueryHandler. + // getResultsLinkPath, which silently mis-resolves and + // fails with a "Resource Error" on a fabricated + // version_XXXX.gord name when this root isn't a real, + // absolute path). This value is not used for access- + // control decisions -- the real enforcement path + // (DriverBackedSecureFileReader) always sources its own + // root independently -- so this only affects path- + // resolution/anchoring correctness, not security scoping. + commonRootOpt = Paths.get("").toAbsolutePath().toString(); + } + if (commonRootOpt.length() > 2 && commonRootOpt.charAt(1) == ':' && !commonRootOpt.endsWith("\\")) { // windows path hack commonRootOpt = commonRootOpt + '\\'; - } else if (!commonRootOpt.endsWith("/")) { + } else if (!commonRootOpt.endsWith("/") && !commonRootOpt.endsWith("\\")) { commonRootOpt = commonRootOpt + '/'; } } diff --git a/model/src/main/java/org/gorpipe/gor/model/DriverBackedFileReader.java b/model/src/main/java/org/gorpipe/gor/model/DriverBackedFileReader.java index b5667a7fe..d5145a5fd 100644 --- a/model/src/main/java/org/gorpipe/gor/model/DriverBackedFileReader.java +++ b/model/src/main/java/org/gorpipe/gor/model/DriverBackedFileReader.java @@ -65,7 +65,18 @@ public class DriverBackedFileReader extends FileReader { private static final Logger log = LoggerFactory.getLogger(DriverBackedFileReader.class); - private static final String DEFAULT_COMMON_ROOT = "./"; + // The process's actual absolute CWD, not the literal string "./" -- + // resolving a relative path against "./" is a no-op in this + // codebase's URI-based path resolution (PathUtils.resolve), so + // anything needing real anchoring against this default (e.g. PGOR's + // dictionary-folder write caching -- see GorSessionFactory. + // updateCommonRoot()'s own matching fix and docstring) quietly + // stayed relative/un-anchored instead. Not used for access-control + // decisions -- this class's own validateAccess() is a no-op, and the + // real enforcement path (DriverBackedSecureFileReader) always + // sources its root independently -- so this only affects path- + // resolution/anchoring correctness, not security scoping. + private static final String DEFAULT_COMMON_ROOT = PathUtils.markAsFolder(Paths.get("").toAbsolutePath().toString()); final static int GZIP_BUFFER_SIZE = Integer.parseInt(System.getProperty("gor.gzip.buffer.size", "2046"));