Hi! Two related observations to sanity-check — might turn into follow-up issues depending on your take.
I came across this while working on the RobotCode REPL. The REPL builds a tiny in-memory suite from user input and lets people experiment with keywords, including Import Resource / Import Library / Import Variables for pulling things in on the fly. I did the initial implementation against RF 7.4, and once everything was working there I went back to check what I'd need to adjust to keep older RF versions backwards-compatible. That's where the trouble started: things that worked cleanly on 7.4 suddenly failed on 7.3 and below, even though I hadn't touched anything import-related.
1. The 7.4 behavior change
The specific shape of the failure was a bare relative path in one of the runtime import keywords. For example, with foo/my.resource sitting next to the suite file:
*** Test Cases ***
first
Import Resource foo/my.resource
My KW
On RF 7.4 this just works. On RF 7.3 and below, the same line fails with Resource file 'foo/my.resource' does not exist. The same goes for the path-style forms of Import Library and Import Variables. Verified across all versions on py3.14:
| RF |
Import Resource |
Import Library (path) |
Import Variables (path) |
| 5.0.1 – 7.3.2 |
❌ |
❌ |
❌ |
| 7.4.2 |
✅ |
✅ |
✅ |
The change came in 7.4 via b4003c9 ("Add owner info to dynamic imports", driven by #5492). Once the runtime imports have an owner, find_file also resolves relative paths against the directory of the file using the keyword — matching the Settings-table Resource / Library / Variables imports.
Now the part that surprised me: in the libdoc of the released RF 7.4.2 (and in fact in every version going back — the docstring is byte-identical from 5.0.1 to 7.4.2), the three BuiltIn keywords still state:
The given path must be absolute or found from [search path].
…and only show ${CURDIR}/... or pythonpath-style examples. That matched the implementation up to 7.3, but on 7.4+ it doesn't anymore.
Question: was the new resolution behavior intentional, or just a side effect of b4003c9? Assuming it's meant to stay, the three BuiltIn docstrings should be reworded to describe the actual 7.4+ contract — something along the lines of:
The given path can be absolute, relative to the directory of the file using this keyword, or found from [search path].
plus an additional bare-relative example (e.g. | Import Resource | resource.txt |) so the new form is visible at a glance.
2. Small side note: sys.path escape via .. in _find_relative_path
While digging into the above, I noticed something tangentially related in robot/utils/robotpath.py:
def _find_relative_path(path, basedir):
for base in [basedir, *sys.path]:
if not (base and os.path.isdir(base)):
continue
...
ret = os.path.abspath(os.path.join(base, path))
if _is_valid_file(ret):
return ret
Because the candidate is just abspath(join(base, path)) with no containment check, a path containing .. can climb out of the sys.path entry and find files anywhere on the filesystem.
Constructed example. Imagine some Python project myproject is installed in editable mode (pip install -e .) on the developer's machine. It uses the standard src-layout, so its directory looks like this:
~/work/myproject/
├── src/ ← added to sys.path by the editable install
│ └── myproject/
└── tests/
└── helpers.resource (some keyword "My KW" that logs "from myproject")
Now imagine anyone else — someone with nothing to do with myproject — is in some unrelated directory /tmp/scratch/ and writes a quick test against a sibling tests/ directory they have, or expect to have, of their own:
/tmp/scratch/dummy.robot:
*** Test Cases ***
first
Import Resource ../tests/helpers.resource
My KW
What they expect: a clean failure. There is no /tmp/tests/helpers.resource, and /tmp/scratch/ itself isn't on sys.path.
What actually happens: _find_relative_path ends up iterating sys.path regardless (because basedir=None on ≤ 7.3, or because the basedir-relative attempt /tmp/tests/helpers.resource doesn't exist on 7.4+). It hits the editable-install entry ~/work/myproject/src, computes
os.path.abspath(os.path.join('~/work/myproject/src', '../tests/helpers.resource'))
→ '~/work/myproject/tests/helpers.resource'
…finds that file, and uses it — a wholly unrelated file from a wholly unrelated project, logging from myproject. Verified on both ≤ 7.3 and 7.4.2; the b4003c9 change from section 1 does not fix this.
That's surprising in two ways:
- Practical / least-surprise: test authors expect
sys.path to be a set of search roots, not "search roots plus anything reachable via ..". The wrong file getting imported silently is hard to debug, and it makes test resolution depend on the global Python environment in non-obvious ways.
- Mild security angle: conceptually the same shape as path traversal in web servers. The threat model is weak (test authors can already do anything), but resource/library import names can come from variables / args / env, i.e. less trusted sources.
For settings-table imports the .. behavior is desired (Resource ../shared/utils.resource relative to the importing file is idiomatic). For sys.path fallback entries it arguably isn't. A possible distinction:
- when
base == setting.directory (file-local): allow ..
- when
base comes from sys.path fallback: require the candidate to stay within base, e.g. os.path.commonpath([candidate, abs_base]) == abs_base
Just floating it as an observation — I realize changing this is backwards-incompatible-ish (some setups may depend on it), so the "right" answer might well be "no, leave it". Mostly wanted to flag that the two pieces (1) and (2) interacted to produce a really confusing debugging session.
Anyway — would love to hear your take on (1), and happy to dig deeper on either of these if it's useful. Sorry for the long read!
Hi! Two related observations to sanity-check — might turn into follow-up issues depending on your take.
I came across this while working on the RobotCode REPL. The REPL builds a tiny in-memory suite from user input and lets people experiment with keywords, including
Import Resource/Import Library/Import Variablesfor pulling things in on the fly. I did the initial implementation against RF 7.4, and once everything was working there I went back to check what I'd need to adjust to keep older RF versions backwards-compatible. That's where the trouble started: things that worked cleanly on 7.4 suddenly failed on 7.3 and below, even though I hadn't touched anything import-related.1. The 7.4 behavior change
The specific shape of the failure was a bare relative path in one of the runtime import keywords. For example, with
foo/my.resourcesitting next to the suite file:On RF 7.4 this just works. On RF 7.3 and below, the same line fails with
Resource file 'foo/my.resource' does not exist.The same goes for the path-style forms ofImport LibraryandImport Variables. Verified across all versions on py3.14:Import ResourceImport Library(path)Import Variables(path)The change came in 7.4 via b4003c9 ("Add owner info to dynamic imports", driven by #5492). Once the runtime imports have an owner,
find_filealso resolves relative paths against the directory of the file using the keyword — matching the Settings-tableResource/Library/Variablesimports.Now the part that surprised me: in the libdoc of the released RF 7.4.2 (and in fact in every version going back — the docstring is byte-identical from 5.0.1 to 7.4.2), the three BuiltIn keywords still state:
…and only show
${CURDIR}/...or pythonpath-style examples. That matched the implementation up to 7.3, but on 7.4+ it doesn't anymore.Question: was the new resolution behavior intentional, or just a side effect of b4003c9? Assuming it's meant to stay, the three BuiltIn docstrings should be reworded to describe the actual 7.4+ contract — something along the lines of:
plus an additional bare-relative example (e.g.
| Import Resource | resource.txt |) so the new form is visible at a glance.2. Small side note:
sys.pathescape via..in_find_relative_pathWhile digging into the above, I noticed something tangentially related in
robot/utils/robotpath.py:Because the candidate is just
abspath(join(base, path))with no containment check, apathcontaining..can climb out of thesys.pathentry and find files anywhere on the filesystem.Constructed example. Imagine some Python project
myprojectis installed in editable mode (pip install -e .) on the developer's machine. It uses the standard src-layout, so its directory looks like this:Now imagine anyone else — someone with nothing to do with
myproject— is in some unrelated directory/tmp/scratch/and writes a quick test against a siblingtests/directory they have, or expect to have, of their own:/tmp/scratch/dummy.robot:What they expect: a clean failure. There is no
/tmp/tests/helpers.resource, and/tmp/scratch/itself isn't onsys.path.What actually happens:
_find_relative_pathends up iteratingsys.pathregardless (becausebasedir=Noneon ≤ 7.3, or because the basedir-relative attempt/tmp/tests/helpers.resourcedoesn't exist on 7.4+). It hits the editable-install entry~/work/myproject/src, computes…finds that file, and uses it — a wholly unrelated file from a wholly unrelated project, logging
from myproject. Verified on both ≤ 7.3 and 7.4.2; the b4003c9 change from section 1 does not fix this.That's surprising in two ways:
sys.pathto be a set of search roots, not "search roots plus anything reachable via..". The wrong file getting imported silently is hard to debug, and it makes test resolution depend on the global Python environment in non-obvious ways.For settings-table imports the
..behavior is desired (Resource ../shared/utils.resourcerelative to the importing file is idiomatic). Forsys.pathfallback entries it arguably isn't. A possible distinction:base == setting.directory(file-local): allow..basecomes fromsys.pathfallback: require the candidate to stay withinbase, e.g.os.path.commonpath([candidate, abs_base]) == abs_baseJust floating it as an observation — I realize changing this is backwards-incompatible-ish (some setups may depend on it), so the "right" answer might well be "no, leave it". Mostly wanted to flag that the two pieces (1) and (2) interacted to produce a really confusing debugging session.
Anyway — would love to hear your take on (1), and happy to dig deeper on either of these if it's useful. Sorry for the long read!