Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ydotool: refactor ; nixos/ydotool: init module & nixosTest #303745

Merged
merged 4 commits into from
May 13, 2024

Conversation

quantenzitrone
Copy link
Contributor

@quantenzitrone quantenzitrone commented Apr 13, 2024

Description of changes

closes #183659

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Apr 13, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Apr 13, 2024
@OPNA2608
Copy link
Contributor

Supersedes #191911
Closes #183659

@quantenzitrone
Copy link
Contributor Author

oh man i should've looked before

Copy link
Contributor

@OPNA2608 OPNA2608 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to get this in, because it'll allow for mouse control in Wayland tests. I've tried this out for Lomiri, and it lets me test some keyboard-unreachable elements.

I really don't feel qualified to judge the correctness of all those service settings however…

nixos/modules/programs/ydotool.nix Outdated Show resolved Hide resolved
nixos/modules/programs/ydotool.nix Outdated Show resolved Hide resolved
@quantenzitrone
Copy link
Contributor Author

@alois31 you added the systemd hardening options on my rimgo module pr so you seem to know some stuff, do you want to review the hardening on this module?

@quantenzitrone
Copy link
Contributor Author

I really don't feel qualified to judge the correctness of all those service settings however…

TBH I just slapped every hardening option that systemd-analyze security suggested on there and checked if ydotool still works. A lot of nixos-rebuild test and reading the systemd.exec manpage.

@OPNA2608
Copy link
Contributor

OPNA2608 commented Apr 16, 2024

Maybe a VM test could be written to verify that the module works as expected? Just loading up a WM or app, using the tool to click on something and verifying via console output / pgrep / OCR that something actually happened.

@quantenzitrone
Copy link
Contributor Author

I have not written a VM test yet, so I will have to read a bit of docs first, but it seems like a good idea.

@quantenzitrone
Copy link
Contributor Author

@OPNA2608 do you have an idea how i could check ydotools keypresses reliably?
i tried using showkey, but the output is unreliable and random.

@quantenzitrone
Copy link
Contributor Author

oh wait you think i should do something in a graphical environment?

@OPNA2608
Copy link
Contributor

OPNA2608 commented Apr 17, 2024

For example yeah (sorry, typo'd earlier). Like using ydotool to click on a starter menu and checking with OCR if any of its contents are displayed. Or just a tty session, sending the inputs for touch somefile\n via ydotool and waiting for the file to appear. But I think mouse usage would be more useful in the case of VM tests.

@alois31
Copy link
Contributor

alois31 commented Apr 17, 2024

@alois31 you added the systemd hardening options on my rimgo module pr so you seem to know some stuff, do you want to review the hardening on this module?

I used the same approach as you did here, and I'm not familiar with ydotool, so I don't think I can be of any help.

@quantenzitrone
Copy link
Contributor Author

another idea i had was directly cating the output of /dev/input/event<number> and comparing the output to a previously recorded version of that output, but no luck, the output does not seem to be reprodible

@OPNA2608
Copy link
Contributor

OPNA2608 commented Apr 23, 2024

I was thinking something like this:

Test file

nixos/tests/ydotool.nix:

import ./make-test-python.nix ({ pkgs, lib, ... }: let
  textInput = "This works.";
in {
  name = "ydotool";

  meta = {
    maintainers = with lib.maintainers; [ OPNA2608 quantenzitrone ];
  };

  nodes = {
    headless = { config, ... }: {
      imports = [
        ./common/user-account.nix
      ];

      users.users.alice.extraGroups = [ "ydotool" ];

      programs.ydotool.enable = true;

      services.getty.autologinUser = "alice";
    };

    x11 = { config, ... }: {
      imports = [
        ./common/user-account.nix
        ./common/auto.nix
        ./common/x11.nix
      ];

      users.users.alice.extraGroups = [ "ydotool" ];

      programs.ydotool.enable = true;

      test-support.displayManager.auto = {
        enable = true;
        user = "alice";
      };

      services.xserver.windowManager.dwm.enable = true;
      services.displayManager.defaultSession = lib.mkForce "none+dwm";

      environment = {
        etc."gui-text".text = textInput;
        systemPackages = with pkgs; [ gnome.zenity ];
      };
    };

    wayland = { config, ... }: {
      imports = [
        ./common/user-account.nix
        ./common/auto.nix
        ./common/x11.nix
      ];

      users.users.alice.extraGroups = [ "ydotool" ];

      programs.ydotool.enable = true;

      test-support.displayManager.auto = {
        enable = true;
        user = "alice";
      };

      programs.miriway.enable = true;
      services.displayManager.defaultSession = lib.mkForce "miriway";

      environment = {
        etc."gui-text".text = textInput;
        systemPackages = with pkgs; [ gnome.zenity ];
      };
    };
  };

  enableOCR = true;

  testScript = { nodes, ... }: ''
    def as_user(cmd: str):
      """
      Return a shell command for running a shell command as a specific user.
      """
      return f"sudo -u alice -i {cmd}"

    def ask_for_input():
      """
      Return a command to spawn a window that asks for input. The input will be sent to /tmp/output.
      """
      return as_user("zenity --entry --text 'Enter input:' > /tmp/output &")

    start_all()

    # Headless
    headless.wait_for_unit("multi-user.target")
    headless.wait_for_text("alice")
    headless.succeed(as_user("ydotool type 'echo ${textInput} > /tmp/output'")) # text input
    headless.succeed(as_user("ydotool key 28:1 28:0")) # text input
    headless.screenshot("headless_input")
    headless.wait_for_file("/tmp/output")
    headless.wait_until_succeeds("grep '${textInput}' /tmp/output") # text input

    # X11
    x11.wait_for_x()
    x11.execute(ask_for_input())
    x11.wait_for_text("Enter input")
    x11.succeed(as_user("ydotool type -f /etc/gui-text")) # text input
    x11.screenshot("x11_input")
    x11.succeed(as_user("ydotool mousemove -a 400 110")) # mouse input
    x11.succeed(as_user("ydotool click 0xC0")) # mouse input
    x11.wait_for_file("/tmp/output")
    x11.wait_until_succeeds("grep '${textInput}' /tmp/output") # text input

    # Wayland
    wayland.wait_for_x()
    wayland.execute(ask_for_input())
    wayland.wait_for_text("Enter input")
    wayland.succeed(as_user("ydotool type -f /etc/gui-text")) # text input
    wayland.screenshot("wayland_input")
    wayland.succeed(as_user("ydotool mousemove -a 350 200")) # mouse input
    wayland.succeed(as_user("ydotool click 0xC0")) # mouse input
    wayland.wait_for_file("/tmp/output")
    wayland.wait_until_succeeds("grep '${textInput}' /tmp/output") # text input
  '';
})

+ a corresponding entry in nixos/tests/all-tests.nix

This tests keyboard functionality on tty, X11 & Wayland, and mouse functionality on X11 & Wayland.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 3, 2024
@OPNA2608
Copy link
Contributor

OPNA2608 commented May 6, 2024

@quantenzitrone Bump on the test situation/question, plus there's a changelog merge conflict that needs to be resolved.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label May 8, 2024
@quantenzitrone
Copy link
Contributor Author

Sorry for stalling this so long.
I changed the wayland test to use cage, because I didn't want to build miriway (which apparently is not in the binary cache even tho I always rebase my PRs to nixos-unstable instead of master).

@quantenzitrone quantenzitrone changed the title nixos/ydotool: init module ydotool: move to pkgs/by-name, add meta.mainProgram, refactor; nixos/ydotool: init module; nixosTests.ydotool: init May 8, 2024
@quantenzitrone quantenzitrone changed the title ydotool: move to pkgs/by-name, add meta.mainProgram, refactor; nixos/ydotool: init module; nixosTests.ydotool: init ydotool: refactor ; nixos/ydotool: init module & nixosTest May 8, 2024
@OPNA2608
Copy link
Contributor

OPNA2608 commented May 8, 2024

No problem!

I didn't want to build miriway (which apparently is not in the binary cache even tho I always rebase my PRs to nixos-unstable instead of master)

A major dependency of it is currently broken by staging fallout: #310091

I changed the wayland test to use cage

A kiosk-like compositor like cage seems like a better fit anyway, so it's fine.

@OPNA2608
Copy link
Contributor

Using the cage test setup doesn't seem to work on aarch64-linux, confirmed on my hardware:

wayland: starting vm
mke2fs 1.47.0 (5-Feb-2023)
qemu-system-aarch64: Virtio VGA not available
[hangs indefinitely]

Seems to fail on this setting:

virtualisation = {
qemu.options = [ "-vga virtio" ];
};

You could try messing with overriding this setting in the test (I don't know why it's there, maybe it's not needed anymore?), or setting up cage without that module.

Copy link
Contributor

@OPNA2608 OPNA2608 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still don't know what all those service settings do, but if systemd thinks it's good & the service still works fine in the VM test, then it can't be completely wrong. LGTM.

@quantenzitrone
Copy link
Contributor Author

I mean, if you want to understand the options, you could read the relevant parts of the systemd.exec(5) and systemd.resource-contro(5) man pages.
systemd-analyze security ydotoold.service also gives a short explanation for every option that is taken into consideration for the security score.

@OPNA2608
Copy link
Contributor

(Merge conflict on release notes again)

@OPNA2608 OPNA2608 added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 12, 2024
@OPNA2608 OPNA2608 removed the 2.status: merge conflict This PR has merge conflicts with the target branch label May 13, 2024
@quantenzitrone
Copy link
Contributor Author

(i forgor that i have to git fetch before git rebaseing)

Copy link
Contributor

@OPNA2608 OPNA2608 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Added a finishing dot to the release notes entry myself, for consistency with the other entries. Just so we don't need another back n forth for such a nit)

@OPNA2608 OPNA2608 merged commit 068c0e3 into NixOS:master May 13, 2024
7 of 8 checks passed
@quantenzitrone quantenzitrone deleted the ydotool branch May 13, 2024 18:00
@natsukium natsukium mentioned this pull request Jun 4, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add programs.ydotool.enable config option to run required ydotoold daemon
4 participants