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

trilium: add desktop item #3

Merged
merged 2 commits into from
Dec 30, 2018

Conversation

emmanuelrosa
Copy link

This change adds a .desktop file so that Trilium can be launched
from desktop menu systems.

Emmanuel Rosa and others added 2 commits December 30, 2018 19:44
This change adds a .desktop file so that Trilium can be launched
from desktop menu systems.
@dtzWill
Copy link
Owner

dtzWill commented Dec 30, 2018

LGTM, thanks!

Didn't notice--for whatever reason rofi managed to find it anyway, but I'm not sure how :).
(it seems likely to only work after running Trilium, however it does it, without this change)

Added nit commit.

FWIW beta for 0.27 is out! :)

@dtzWill
Copy link
Owner

dtzWill commented Dec 30, 2018

Since we're using this, it seems it'd be better to submit upstream? I'll put that together in a few...

@dtzWill dtzWill merged commit de0cf39 into dtzWill:experimental/trilium Dec 30, 2018
@emmanuelrosa
Copy link
Author

Yes, to my surprise rofi found Trilium. However in my case that menu item didn't work. I discovered a .desktop file for Trilium in $HOME/.local/share/applications, and I'm assuming Trilium created it.

Oh, 0.27 claims to have a responsive UI. Yes!

@emmanuelrosa
Copy link
Author

Since we're using this, it seems it'd be better to submit upstream? I'll put that together in a few...

Agreed. Having been using this successfully for a few weeks, I trust the package is ready to go upstream.

I've actually been using it as an overlay rather than from this branch.

dtzWill pushed a commit that referenced this pull request Feb 8, 2022
The test failed with

> Test "test5 user should not be able to run commands under root" failed with
> error: "invalid literal for int() with base 10: ''"

since 2492da8.

The reason for this is that `sudo(8)` writes the lecture to the
tty[1] and only as a fallback to stdout[2]. This means that the
`base64 --wrap 0` executed by `machine.execute()` doesn't affect the
text written to the terminal, however the lecture is part of the string
that's read from the VM via `shell.recv()`.

I confirmed the problem in an interactive test session[3]:

    >>> command = "sudo -u test5 sudo -n -u root true"
    >>> out_command = f"( set -euo pipefail; {command} ) | (base64 --wrap 0; echo)\n"
    >>> machine.shell.send(out_command.encode())
    84

    >>> machine # [   99.015512] sudo[877]:     root : TTY=hvc0 ; PWD=/tmp ; USER=test5 ; COMMAND=/run/wrappers/bin/sudo -n -u root true
    machine # [   99.019373] sudo[877]: pam_unix(sudo:session): session opened for user test5(uid=1005) by (uid=0)
    machine # [   99.038692] sudo[879]: pam_unix(sudo:auth): conversation failed
    machine # sudo: a password is required
    machine # [   99.041860] sudo[879]: pam_unix(sudo:auth): auth could not identify password for [test5]
    machine # [   99.046901] sudo[877]: pam_unix(sudo:session): session closed for user test5
    >>>
    >>> x=machine._next_newline_closed_block_from_shell()
    >>> print(x)
    <newline>
    We trust you have received the usual lecture from the local System
    Administrator. It usually boils down to these three things:
    <newline>
        #1) Respect the privacy of others.
        #2) Think before you type.
        #3) With great power comes great responsibility.
    <newline>
    <newline>
    <newline>
    >>>

Since the lecture isn't strictly necessary to confirm that
`security.sudo` works as expected, I decided to disable lecturing
inside the test, however we may want to fix the underlying problem in
the test-driver at some point.

[1] https://github.com/sudo-project/sudo/blob/SUDO_1_9_9/plugins/sudoers/check.c#L275-L283
[2] https://github.com/sudo-project/sudo/blob/SUDO_1_9_9/src/conversation.c#L95-L120
[3] I replaced each empty line with `<newline>` to make sure these
    aren't swallowed by git.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants