Skip to content

Question: Clarify relative-path resolution for dynamic imports in RF 7.4 #5682

Description

@d-biehl

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!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions