workbench: init at 43.3

[onny]

Description of changes

Packaging Workbench, a Gnome software for prototyping GTK4 applications

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.05 Release Notes (or backporting 22.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[onny]

Unfortunately I get following error when trying to compile workbench

       > [28/30] Compiling C object src/Previewer/workbench-vala-previewer.p/meson-generated_previewer.c.o
       > [29/30] Linking target src/Previewer/workbench-vala-previewer
       > [30/30] Generating src/gjspack with a custom command
       > FAILED: src/re.sonny.Workbench.src.gresource
       > /build/source/src/../troll/gjspack/bin/gjspack --appid=re.sonny.Workbench --prefix /re/sonny/Workbench --resource-root /build/source --blueprint-compiler /nix/store/2p9zl4sz7ndvi5qm5fb760k8irnnwkn8-blueprint-compiler-0.6.0/bin/blueprint-compiler --no-executable ../src/main.js --potfiles ../src/../po/POTFILES src
       > ninja: build stopped: subcommand failed.

There is no further output why this command is failing.

Can someone familiar with packaging GTK Vala apps help out with this issue? @austinbutler @chuangzhu @hqurve

[chuangzhu]

This also struggled me when I was packaging Tangram 2.0. gjspack didn't print anything when its subprocesses failed. We definitely want to PR the upstream to make it verbose. But for now, I did a cd $(mktemp -d); nix-shell ~/code/nixpkgs -A workbench -c genericBuild and edited the gjspack.js embedded in ../troll/gjspack/bin/gjspack.src.gresource to make it print failed commands (Hint: the transform function), and this is what I got:

/nix/store/2p9zl4sz7ndvi5qm5fb760k8irnnwkn8-blueprint-compiler-0.6.0/bin/blueprint-compiler compile /tmp/tmp.k1H4QyMpRE/source/src/window.blp
/nix/store/2p9zl4sz7ndvi5qm5fb760k8irnnwkn8-blueprint-compiler-0.6.0/bin/blueprint-compiler compile /tmp/tmp.k1H4QyMpRE/source/src/widgets/CodeView.blp

And running them manually I can tell it's because you missed these two dependencies.

[chuangzhu]

The app however fails to launch for the second time:

(re.sonny.Workbench:1111316): Gjs-CRITICAL **: 18:14:49.509: JS ERROR: Gio.IOErrorEnum: Error opening file “/home/chuang/.local/share/re.sonny.Workbench/workbench.vala”: Permission denied

And I think it's because the app tries to copy $out/share/re.sonny.Workbench/workbench-api.vala to ~/.local/share/re.sonny.Workbench/workbench.vala on start, but files in Nix store has a 444 mode so it fails at the second run:

[chuang@hawthorn:~]$ ls .local/share/re.sonny.Workbench/workbench.vala -l
-r--r--r-- 1 chuang users 363 Jan  1  1970 .local/share/re.sonny.Workbench/workbench.vala

[onny]

Thank you @chuangzhu this looked more complicated than I thought. Maybe there are still some patches required to get it working correctly

[hqurve]

And I think it's because the app tries to copy $out/share/re.sonny.Workbench/workbench-api.vala to ~/.local/share/re.sonny.Workbench/workbench.vala on start, but files in Nix store has a 444 mode so it fails at the second run:

By searching the source for "workspace.vala", I determined that the file copy is performed at https://github.com/sonnyp/Workbench/blob/v43.3/src/langs/vala/vala.js#L13. By looking at the GIO api for the copy function, we can use the G_FILE_COPY_TARGET_DEFAULT_PERMS flag to "Leave target file with default perms, instead of setting the source file perms."

I applied the following patch and rebuilt workbench. After clearing the ~/.local/share/re.sonnyp.Workbench directory, reopening worked

From b5862ec862a1e9a4b01b09f3ca341e57999353c3 Mon Sep 17 00:00:00 2001
From: hqurve <[email protected]>
Date: Wed, 8 Feb 2023 07:31:04 -0400
Subject: [PATCH] do not copy source perms

---
 src/langs/vala/vala.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/langs/vala/vala.js b/src/langs/vala/vala.js
index e5ccfcc..dc0b25b 100644
--- a/src/langs/vala/vala.js
+++ b/src/langs/vala/vala.js
@@ -12,7 +12,7 @@ export function setup({ data_dir, document }) {
 
   api_file.copy(
     Gio.File.new_for_path(data_dir).get_child("workbench.vala"),
-    Gio.FileCopyFlags.OVERWRITE,
+    Gio.FileCopyFlags.OVERWRITE | Gio.FileCopyFlags.TARGET_DEFAULT_PERMS,
     null,
     null,
   );
-- 
2.39.1

I assume that flags work using bits. Also, you probably want to reword the commit message.

Also, by searching the code for copy(, I think this is the only instance of copying files

[jtojnar]

Was the removal of the dependencies intentional?

[jtojnar]

Would be nice to upstream this.

[onny]

Created an upstream PR workbenchdev/Workbench#188

Going to use this as a patch for now

[onny]

Was the removal of the dependencies intentional?

Yes I copied the Nix expression for the package from an other package, these dependencies weren't meant to be added

[AndersonTorres]

LGTM

[onny]

I added the patch by @hqurve from the upstream PR

  patches = [
    # Do not copy files with source permissions into home directory while
    # running Workbench, see https://github.com/sonnyp/Workbench/pull/188
    (fetchpatch {
      url = "https://github.com/sonnyp/Workbench/commit/c553f932c952057674c3333e4af84044153b76d7.patch";
      hash = "sha256-pFj2LvqJE7uGhCYs6GL2a7tj4n2BBoKcPzD1K1JZ3Rs=";
      name = "do-not-copy-source-perms.patch";
    })
  ];

But somehow I get

       last 10 log lines:
       > Perhaps you used the wrong -p or --strip option?
       > The text leading up to this was:
       > --------------------------
       > |--- src/langs/vala/vala.js
       > |+++ src/langs/vala/vala.js
       > --------------------------
       > File to patch:
       > Skip this patch? [y]
       > Skipping patch.
       > 1 out of 1 hunk ignored
       For full logs, run 'nix log /nix/store/jxk6ncv5r5i08a8n3r9199d8qdr34na9-workbench-43.3.drv'.

Patch was applied to the correct and latest version of Workbench :/

[sonnyp]

Workench author here.

I appreciate people being interested in Workbench but I really don't think it's a good idea to package Workbench

https://github.com/sonnyp/Workbench#packaging

Please do not attempt to package Workbench any other way than as a Flatpak application.
It is unsupported and may put users at risk.

There are 2 main issues

1. Workbench allows arbitrary code execution (paste and run). This might become even more of an issue in non-sandbox env with Add an option to share a Workbench URI workbenchdev/Workbench#181 which will allow "Run in Workbench" on the Web.

2. Workbench Library increasingly relies on a specific GNOME Platform - demos and examples will keep breaking

Maybe the warning isn't visible enough? Perhaps I should exit with an error if not running in a sandbox.

WDYT?

[hqurve]

But somehow I get

       last 10 log lines:
       > Perhaps you used the wrong -p or --strip option?
       > The text leading up to this was:
       > --------------------------
       > |--- src/langs/vala/vala.js
       > |+++ src/langs/vala/vala.js
       > --------------------------
       > File to patch:
       > Skip this patch? [y]
       > Skipping patch.
       > 1 out of 1 hunk ignored
       For full logs, run 'nix log /nix/store/jxk6ncv5r5i08a8n3r9199d8qdr34na9-workbench-43.3.drv'.

When I ran the build, I got a hash mismatch for the patch (you probably forgot to update the hash). Updating the hash caused the program to build. Updated hash: sha256-oV7cAUjzkQjIdrMRzJAn1sO6z4fYLXYOdcFSkUoGCO8=

[hqurve]

(Although I am not qualified to comment .... I still will.)

At first I thought the app was an alternative to gnome builder. But after reading more, I realize it is more like a gtk version of https://jsfiddle.net which is made for sharing/testing ideas. Also, it runs locally....

After realizing this, I think there should be some sandbox around the application so that inexperienced people cannot accidentally run dangerous code that they found online. The same argument could probably be extended to other applications such as a terminal. But Workbench's case is special since the there is no need for it to access the whole system and application was designed with the idea of it being in a sandbox.

Instead of not packaging it or packaging it without a sandbox, we could possibly package it with bubblewrap around it(?)

[sonnyp]

The whole point is to have a very specific target env, so Flatpak + org.gnome.Sdk/org.gnome.Platform

It might sandbox but things will still break. Portals and permissions will be expected to work.

[jtojnar]

At minimum, you will want wrapGAppsHook4. And probably whatever libraries are is in GNOME platform to make it more useful.

[Antiz96]

Maybe the warning isn't visible enough? Perhaps I should exit with an error if not running in a sandbox.

WDYT?

Hi @sonnyp, I'm the one that rejected your deletion request for the workbench-git AUR package.

If running workbench outside of a sandbox environment is a concern, I think you should indeed make a more detailed warning or eventually consider the exit/error if not running in a sandbox.
While we get the initial concern, workbench is a free software licensed under the GPL license. According to that, there's not much we can do to prevent people redistributing workbench as they please. We can only assume that people read the related warning on the upstream repo before installing workbench from the AUR.

[sonnyp]

If running workbench outside of a sandbox environment is a concern, I think you should indeed make a more detailed warning or eventually consider the exit/error if not running in a sandbox.

Thanks for the feedback – I will proceed with that and let users know of the risks.

While we get the initial concern, workbench is a free software licensed under the GPL license. According to that, there's not much we (nor you) can do to prevent people redistributing workbench as they please.

I am aware of Workbench license and its conditions. This isn't a licensing issue.

Who is "we"? Just like with aur - my request is directed at the packager.

We can only assume that people read the related warning on the upstream repo before installing workbench from the AUR.

If packagers don't – it's hard to assume users will.

I won't insist further.

---

Side note: Let's please not turn this into a drama/crusade 🙏

This is a very practical request that has the best interests of users in mind.

I am hopeful packagers can see and share that.

If anyone is willing to brainstorm/collaborate on making non-flatpak Workbench safe for all – you are welcome to file an issue.

[chuangzhu]

Flatpak = a packaging system + bubblewrap + xdg-dbus-proxy + xdg-desktop-portal. It's possible for us to do the exact same runtime isolation as Flatpak. There is, for example, an out-of-tree module called NixPak. Maybe we need to make a in-tree equivalent of that in Nixpkgs.

[Antiz96]

While we get the initial concern, workbench is a free software licensed under the GPL license. According to that, there's not much we (nor you) can do to prevent people redistributing workbench as they please.

I am aware of Workbench license and its conditions. This isn't a licensing issue.

Who is "we"? Just like with aur - my request is directed at the packager.

"We" refers as ArchLinux trusted users (AUR moderators). AUR requests (deletion/merge/orphan) are directed to us for a review and an eventual acceptance. If you wish to contact the packager, you can do so by leaving a comment on the associated AUR page or sending them an email.
About the "licencing issue" part, what I meant is that the workbench-git AUR package does not go against AUR submission guidelines nor upstream license, so there's not much we (AUR moderators) can do regarding the deletion request.

We can only assume that people read the related warning on the upstream repo before installing workbench from the AUR.

If packagers don't – it's hard to assume users will.

They are expected to though. For what it's worth, AUR packages are not officially supported by Arch and, most of the time, neither by upstream developers. As stated officially in the AUR' Arch wiki page, AUR users should be aware that they are using AUR packages at their own risks and are thus implicitly expected to read upstream documentation for such eventual warnings.
However, we (AUR moderators) cannot prevent users from submitting PKGBUILDs/packages to the AUR unless they do not respect the AUR submission guidelines or the upstream license (which is not the case here).

---

Side note: Let's please not turn this into a drama/crusade 🙏

This is a very practical request that has the best interests of users in mind.

My sole intention here is to help regarding the current situation :)

TLDR: People are expected to read upstream documentation before installing software so they should be aware of the potential risks of running workbench outside of a sandbox. Regarding the AUR specifically, they should also be aware that AUR packages are not officially supported.
If you really want to prevent users from packaging/using workbench outside of a sandbox, I assume you have to prevent it either via the license or technically within the software (error/exit if not run inside a sandbox). As of now, since the workbench AUR package respects the submission guidelines as well as the workbench GPL license, there's not much we can do from a moderation point of view (despite your legit warning, unfortunately).

EDIT: Sorry for going a bit off-topic with this MR... @sonnyp we can discuss this further somewhere else if you want to.
EDIT2: Since the maintainer of the workbench-git AUR package themself made another deletion request regarding the sandbox warning, I deleted the package 😉
However, the above is still true, as of now we (AUR moderators) can't prevent someone else to re-submit it in the future unfortunately :/

[sund3RRR]

Hi guys. I just decided to try running the latest version and see what happens.

And I would like to start with my opinion on this situation. There are many ways in the world to destroy or infect your computer. Expecially in most linux distros you can simply run sudo rm -rf / and destroy not only your system, but also your PC firmware, if you're unlucky.

If we talk about the Workbench, then the target audience of this application is not ordinary users, these are developers. Who else, if not developers, should know about caution on the Internet?

And even if we move away from this thought and keep the sandbox for greater security, as discussed above, the app lacks a warning feature to let you know it's not running in a sandbox. I decided to look at the source and saw this there:

if (!Xdp.Portal.running_under_flatpak()) {
  console.error(
    "Flatpak required\nWorkbench is only meant to be run sandboxed in a specific target environment.\nBypassing this will exposes users to arbitrary code execution and breakage.",
  );
  exit(1);
}

Is flatpak == sandboxing?

This application is packed even without any specific patches in nix, and if you remove this limitation, then the application starts up and runs smoothly. Even if the execution of the code breaks because of the platform, this is already a problem for the packagers, not the application.

In my opinion, there are no restrictions for running a Workbench outside of a flatpak, if necessary, you can always screw a nixpak.

Linux has always been about freedom and choice... Is there less choice now?

[Aleksanaa]

(.re.sonny.Workbench-wrapped:529736): .re.sonny.Workbench-wrapped-CRITICAL **: 17:19:43.028: Flatpak required
Workbench is only meant to be run sandboxed in a specific target environment.
Bypassing this will exposes users to arbitrary code execution and breakage.

https://github.com/workbenchdev/Workbench/blob/b16fc2ddecd7c4eab28312cddb7aea3fbf033fdf/src/bin.js#L21

Obviously the author doesn't want this program to run outside of flatpak (although the reason is far-fetched). We may not want to workaround this.