Skip to content

marketplace install refuses every package file when -p is a bare .mpr filename, and leaves the project half-installed #602

Description

@ako

Summary

mxcli marketplace install refuses to write any of a package's bundled files when -p names the .mpr with no directory component:

install the package's bundled files: package entry
"javasource/workflowcommons/actions/JA_UserTaskView_GenerateKey.java" would write outside the project

The message points at the package, and at a javasource path in particular. Neither is at fault — the trigger is the shape of -p, and the named entry is just the first one in the zip to reach the guard.

Worse than the refusal: by the time it fires, the module has already been transplanted into the model. The command exits 1 with the project half-installed — module present, none of its files on disk — and neither mx check nor a full deploy build says anything about it.

Reproduction

Two identical clean projects, same package, only -p differs:

mxcli new GuardRel --version 11.12.4 --theme none --layout none --skip-init --output-dir RelTest
cp -R RelTest AbsTest

cd RelTest && mxcli marketplace install 117066 --version 4.10.0 -p GuardRel.mpr
#   install the package's bundled files: package entry "javasource/…/JA_UserTaskView_GenerateKey.java"
#   would write outside the project
#   exit 1

cd ../AbsTest && mxcli marketplace install 117066 --version 4.10.0 -p "$PWD/GuardRel.mpr"
#   succeeds — javasource/workflowcommons/actions/JA_UserTaskView_GenerateKey.java is written

Content id 117066 is Workflow Commons; the package is incidental, any package with bundled files does it.

-p projectDir result
GuardRel.mpr "." refused
./GuardRel.mpr "." refused
sub/GuardRel.mpr "sub" ok
/abs/path/GuardRel.mpr "/abs/path" ok

Root cause

cmd/mxcli/marketplace/update.go:183 and :431 pass filepath.Dir(mprPath) as projectDir. For a bare filename that is ".", and the containment check in InstallPackageFiles (update.go:363-366) then cannot pass:

dst := filepath.Join(projectDir, clean)
if !strings.HasPrefix(filepath.Clean(dst), filepath.Clean(projectDir)+string(os.PathSeparator)) {
    return written, skipped, fmt.Errorf("package entry %q would write outside the project", f.Name)
}

With projectDir == ".", filepath.Join(".", "javasource/x.java") returns "javasource/x.java" — Join cleans the ./ away — while the required prefix is "./". No entry can ever match, so the first file in the archive fails.

Isolated, with everything else held constant:

-p GuardRel.mpr        => projectDir "."  REFUSED  dst="javasource/…/JA_….java", want prefix "./"
-p sub/GuardRel.mpr    => projectDir "sub"  OK
-p /abs/proj/Guard.mpr => projectDir "/abs/proj"  OK

The entry-name check just above it (strings.HasPrefix(clean, "..")) is fine — this is only the joined-path check, which the comment says exists in that shape so CodeQL's go/zipslip query recognises it as a sanitizer.

Note filepath.Dir(mprPath) → "." is a common idiom across the codebase (cmd_brain.go, cmd_widget.go, devloop_handshake.go, tui/app.go, …) and is harmless everywhere else, because those only join — filepath.Join(".", x) resolves against the cwd correctly. This guard is the one place that compares against the joined path, so it is the only site affected. Fixing it here is preferable to normalising -p at every caller.

Why the tests miss it

install_traversal_test.go and widgetversion_test.go build projectDir from t.TempDir(), which is always absolute. The suite's own control — "a legitimate entry must actually be written, so a test that nothing escaped is not passing because nothing was extracted at all" — is exactly the right control and still cannot see this, because a relative projectDir has never been exercised. A case with projectDir of "." would catch it.

The half-installed state

This is what makes it more than a usability wart. After the failing run:

$ mxcli -p GuardRel.mpr -c "show modules" | grep -i workflow
| WorkflowCommons | Marketplace v4.10.0 | 35 | 7 | 42 | 84 | 177 | 21 | 0 | 2 | 1 | …

$ ls javasource/workflowcommons
ls: cannot access 'javasource/workflowcommons': No such file or directory

$ ls themesource/
administration  atlas_core  atlas_web_content  datawidgets  feedbackmodule  myfirstmodule  nanoflowcommons

The model carries 35 entities, 177 microflows and a declared Java action; none of javasource/, themesource/ or widgets/ for the module is on disk. Re-running the install then reports "Module WorkflowCommons is already installed (version 4.10.0)" and declines, so the obvious retry does not repair it.

Neither check catches the state: mx check reports 0 errors, and a full mxcli docker build (target=portable-app-package) succeeds. So the only signal a user gets is the one confusing error line.

Whether the model transplant should roll back when the file install fails is arguably a separate concern from the guard itself, but it is what turns this from "run it again with a different path" into a project that needs manual repair.

Suggested fix

Make the containment check work for a relative projectDir — resolve both sides before comparing (filepath.Abs, or compare with filepath.Rel and reject a result starting with ..), keeping a shape CodeQL still reads as a sanitizer. A regression case with projectDir == "." alongside the existing absolute one.

Separately worth considering: not leaving the model updated when the file install fails.

Environment

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions