Feature/plugins #56

vikanezrimaya · 2024-01-26T18:32:34Z

API surface exposed is equivalent to the Rust API, including plugin callbacks (exposed as pyrage.plugin.Callbacks, which must be subclassed for anything interesting to happen).

Non-Cloneability of plugin instances is resolved by putting them behind an Arc. Perhaps a more elegant solution could be produced later, by someone with more knowledge of PyO3 internals.

This was tested with rage's example age-unencrypted-plugin which happens to exercise the display_message callback, and additionally with age-plugin-tpm to exercise actual encryption functionality.

Typing stubs are amended to cover the new API surface.

Resolves #55.

EDIT 2024-03-29: now based on #63, which should be merged first.

Cargo.lock

src/plugin.rs

woodruffw · 2024-01-28T22:34:46Z

src/plugin.rs

+/// Hack, because the orphan rule would prevent us from deriving a
+/// foreign trait on a foreign object. Instead, define a newtype.
+#[derive(Clone)]
+pub(crate) struct PyCallbacks(Py<Callbacks>);
+
+#[pymethods]
+#[allow(unused_variables)]
+impl Callbacks {


Just leaving a note: I need to think about this a bit. Rather than messing around with subclassing like this, we may just want the newtype to be over Py<PyAny> and do structural subtyping instead (since we get relatively little from the concrete base here).

What a coincidence, I also entertained the thought of just duck-typing everything, but I guess I'm a bit rusty (pardon the pun) when it comes to proper types.

I suppose I could rewrite this bit to duck-type on Rust side, and add an interface class to the Python type stubs so that the Python side can still type-check the usage of this API.

I suppose I could rewrite this bit to duck-type on Rust side, and add an interface class to the Python type stubs so that the Python side can still type-check the usage of this API.

Yeah, this sounds good to me -- I think the performance will be identical, and will be a bit easier to reason about/read here 🙂

Rewrote to duck-type and use a protocol. I still explicitly check on construction of the PyCallbacks wrapper, but perhaps this check could be omitted in lieu of static type-checking (which I checked is working properly!)

vikanezrimaya · 2024-01-29T15:31:04Z

Example program that passes the type check and runs successfully:

import pyrage
import typing

class Callbacks:
    def display_message(self, message: str) -> None:
        print(message)

    def confirm(
            self,
            message: str,
            yes_string: str,
            no_string: str | None
    ) -> typing.Optional[bool]:
        print(message)
        if no_string is not None:
            prompt = f"{yes_string}/{no_string}? [y/n]"
        else:
            prompt = f"{yes_string}? [y/n]"
        
        i = None
        while True:
            i = input(prompt)
            if len(i) < 1:
                i = None
                continue
            if i[0] in ["y", "Y"]:
                return True
            if i[0] in ["n", "N"]:
                return False

    def request_public_string(self, description: str) -> typing.Optional[str]:
        i = input(f"{description}: ")
        if i == "":
            return None
        return i

    # XXX use obscured input
    def request_passphrase(self, description: str) -> typing.Optional[str]:
        i = input(f"(passphrase) {description}: ")
        if i == "":
            return None
        return i

with open("/home/vika/age-tpm.txt", "r") as f:
    identity = pyrage.plugin.Identity.from_str(f.read().strip())
    recipient = pyrage.plugin.Recipient.from_str(
        "age1tpm1qv75z0j9pkefmp0etdmcqapxdm9wzze4wl3j5zjn66ksung85wvvchxel2y"
    )

unencrypted_identity = pyrage.plugin.Identity.default_for_plugin("unencrypted")

recipient_unencrypted_plugin = pyrage.plugin.RecipientPluginV1(
    unencrypted_identity.plugin(), [], [unencrypted_identity], Callbacks()
)
identity_unencrypted_plugin = pyrage.plugin.IdentityPluginV1(
    unencrypted_identity.plugin(), [unencrypted_identity], Callbacks()
)

identity_tpm_plugin = pyrage.plugin.IdentityPluginV1(
    identity.plugin(),
    [identity],
    Callbacks()
)
recipient_tpm_plugin = pyrage.plugin.RecipientPluginV1(
    identity.plugin(),
    [recipient],
    [identity],
    Callbacks()
)

assert(pyrage.decrypt(
    pyrage.encrypt(b"test", [recipient_unencrypted_plugin, recipient_tpm_plugin]),
    [identity_unencrypted_plugin, identity_tpm_plugin]
) == b"test")

Output:

$ maturin build && pip install --force-reinstall target/wheels/pyrage-1.1.3rc1-cp38-abi3-manylinux_2_34_x86_64.whl ./pyrage-stubs && PYTHONPATH=venv/lib/python3.11/site-packages:$PYTHONPATH mypy --strict ./test_plugin.py && PATH=$PWD/../rage/target/debug/examples:$PATH python3 test_plugin.py
⚠️  Warning: You specified maturin >=0.14, <0.15 in pyproject.toml under `build-system.requires`, but the current maturin version is 1.4.0
📦 Including license file "/home/vika/nyantec/Projects/operations/pyrage/LICENSE"
🔗 Found pyo3 bindings with abi3 support for Python ≥ 3.8
🐍 Not using a specific python interpreter
warning: unused import: `exceptions::PyAttributeError`
 --> src/plugin.rs:4:12
  |
4 | use pyo3::{exceptions::PyAttributeError, prelude::*, types::PyType};
  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: `pyrage` (lib) generated 1 warning (run `cargo fix --lib -p pyrage` to apply 1 suggestion)
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s
📦 Built wheel for abi3 Python ≥ 3.8 to /home/vika/nyantec/Projects/operations/pyrage/target/wheels/pyrage-1.1.3rc1-cp38-abi3-manylinux_2_34_x86_64.whl
Processing ./target/wheels/pyrage-1.1.3rc1-cp38-abi3-manylinux_2_34_x86_64.whl
Processing ./pyrage-stubs
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Installing backend dependencies ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: pyrage-stubs
  Building wheel for pyrage-stubs (pyproject.toml) ... done
  Created wheel for pyrage-stubs: filename=pyrage_stubs-1.1.0-py3-none-any.whl size=5023 sha256=5a057ebf600dae4b66305148572dbd1fcdacb15572506c8f058bdfa2f11fe55f
  Stored in directory: /home/vika/.cache/pip/wheels/9b/4f/68/e49c2c73e68d037f825b754a56f168d824899d18d730deb26c
Successfully built pyrage-stubs
Installing collected packages: pyrage, pyrage-stubs
  Attempting uninstall: pyrage
    Found existing installation: pyrage 1.1.3rc1
    Uninstalling pyrage-1.1.3rc1:
      Successfully uninstalled pyrage-1.1.3rc1
  Attempting uninstall: pyrage-stubs
    Found existing installation: pyrage-stubs 1.1.0
    Uninstalling pyrage-stubs-1.1.0:
      Successfully uninstalled pyrage-stubs-1.1.0
Successfully installed pyrage-1.1.3rc1 pyrage-stubs-1.1.0

[notice] A new release of pip is available: 23.2.1 -> 23.3.2
[notice] To update, run: pip install --upgrade pip
Success: no issues found in 1 source file
This plugin doesn't have any recipient-specific logic. It's unencrypted!
This identity does nothing!

woodruffw · 2024-01-29T16:15:11Z

Nice work! I'll do another round of review tonight.

src/plugin.rs

woodruffw · 2024-01-30T01:04:08Z

This is looking pretty good, thank you @vikanezrimaya! If you could give age_plugin a try that would be fantastic (and if it doesn't work we can go ahead with the current approach).

Finally, it'd be good to have 1-2 tests in this PR before merge. Nothing huge; just something small like the sample you posted above.

vikanezrimaya · 2024-01-30T01:15:16Z

I'm not really sure how to test it, given it inherently depends on external binaries. This will need research.

vikanezrimaya · 2024-02-02T09:20:09Z

I am terribly sorry for piling up so many unrelated things into this merge request, but I keep finding small things that can be fixed with just a few lines of code, and I'm also using this package for internal tooling at my workplace, so I keep stumbling into things that need to be fixed all the time! If you wish, I could probably split a few commits into separate merge requests so we can merge them independently of everything else.

woodruffw · 2024-03-27T15:31:51Z

Sorry, I've been a big backlogged on reviews. Taking a look again today!

pyrage-stubs/pyproject.toml

vikanezrimaya · 2024-03-29T12:31:05Z

I split off unrelated type-stub fixes into #63 (which was based on top of latest main) and rebased this branch on top of #63. Merging that would push these small fixes faster into the main branch, while I can spend more time musing on how best to create tests for the plugin functionality (sadly, I didn't have too much time to think on that, because of my $dayjob responsibilities)

woodruffw · 2024-04-03T17:47:06Z

Sorry for the hassle here -- I think you'll need to deconflict now that pyo3 has updated to use the Bound<...> APIs.

vikanezrimaya · 2024-04-05T15:25:20Z

Thanks for the ping! That Bound API sure looks nice — I rewrote the plugin code to use it where possible, so we get no deprecation warnings and nicer code in some places.

src/plugin.rs

API surface exposed is equivalent to the Rust API, including plugin callbacks (duck-typed at runtime, exposed in type stubs as a `pyrage.plugin.Callbacks` protocol, all of which's methods are required to be implemented at runtime, otherwise `AttributeError` is thrown). Non-`Clone`ability of plugin instances is resolved by putting them behind an `Arc`. Perhaps a more elegant solution could be produced later, by someone with more knowledge of PyO3 internals. This was tested with rage's example `age-unencrypted-plugin` which happens to exercise the `display_message` callback, and additionally with `age-plugin-tpm` to exercise actual encryption functionality. Typing stubs are amended to cover the new API surface. Conformance to type-stubs was checked by writing a short Python program exercising the functionality and type-checking it using `mypy` with the type stubs installed.

woodruffw

LGTM, thanks for your patience here @vikanezrimaya!

woodruffw · 2024-04-13T00:16:09Z

P.S. let me know if you'd like a release with these new APIs. Otherwise I'll likely do it sometime later this month.

woodruffw reviewed Jan 28, 2024

View reviewed changes

Cargo.lock Outdated Show resolved Hide resolved

woodruffw reviewed Jan 28, 2024

View reviewed changes

src/plugin.rs Outdated Show resolved Hide resolved

woodruffw reviewed Jan 28, 2024

View reviewed changes

vikanezrimaya force-pushed the feature/plugins branch 2 times, most recently from b7f297d to b6ba84f Compare January 29, 2024 15:27

vikanezrimaya requested a review from woodruffw January 29, 2024 15:51

vikanezrimaya force-pushed the feature/plugins branch from b6ba84f to b8e8cd2 Compare January 29, 2024 15:57

woodruffw reviewed Jan 30, 2024

View reviewed changes

src/plugin.rs Outdated Show resolved Hide resolved

woodruffw reviewed Jan 30, 2024

View reviewed changes

src/plugin.rs Show resolved Hide resolved

woodruffw reviewed Jan 30, 2024

View reviewed changes

src/plugin.rs Show resolved Hide resolved

woodruffw reviewed Jan 30, 2024

View reviewed changes

src/plugin.rs Show resolved Hide resolved

woodruffw reviewed Mar 27, 2024

View reviewed changes

pyrage-stubs/pyproject.toml Outdated Show resolved Hide resolved

woodruffw reviewed Mar 27, 2024

View reviewed changes

pyrage-stubs/pyproject.toml Outdated Show resolved Hide resolved

vikanezrimaya mentioned this pull request Mar 29, 2024

Fixes for type stubs #63

Merged

vikanezrimaya force-pushed the feature/plugins branch from 87e9936 to 50506b1 Compare March 29, 2024 12:28

vikanezrimaya mentioned this pull request Mar 29, 2024

Add ASCII Armored Option [Feature Request] #45

Open

vikanezrimaya force-pushed the feature/plugins branch from 50506b1 to 235088c Compare March 29, 2024 14:34

vikanezrimaya force-pushed the feature/plugins branch 2 times, most recently from 1c8895d to 276713f Compare April 5, 2024 15:23

woodruffw reviewed Apr 7, 2024

View reviewed changes

src/plugin.rs Show resolved Hide resolved

woodruffw reviewed Apr 9, 2024

View reviewed changes

src/plugin.rs Outdated Show resolved Hide resolved

vikanezrimaya force-pushed the feature/plugins branch from 276713f to 0467326 Compare April 9, 2024 08:58

woodruffw approved these changes Apr 13, 2024

View reviewed changes

woodruffw merged commit eac37b3 into woodruffw:main Apr 13, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/plugins #56

Feature/plugins #56

vikanezrimaya commented Jan 26, 2024 •

edited

Loading

woodruffw Jan 28, 2024

vikanezrimaya Jan 28, 2024

woodruffw Jan 28, 2024

vikanezrimaya Jan 29, 2024

vikanezrimaya commented Jan 29, 2024

woodruffw commented Jan 29, 2024

woodruffw commented Jan 30, 2024

vikanezrimaya commented Jan 30, 2024

vikanezrimaya commented Feb 2, 2024

woodruffw commented Mar 27, 2024

vikanezrimaya commented Mar 29, 2024

woodruffw commented Apr 3, 2024

vikanezrimaya commented Apr 5, 2024

woodruffw left a comment

woodruffw commented Apr 13, 2024

Feature/plugins #56

Feature/plugins #56

Conversation

vikanezrimaya commented Jan 26, 2024 • edited Loading

woodruffw Jan 28, 2024

Choose a reason for hiding this comment

vikanezrimaya Jan 28, 2024

Choose a reason for hiding this comment

woodruffw Jan 28, 2024

Choose a reason for hiding this comment

vikanezrimaya Jan 29, 2024

Choose a reason for hiding this comment

vikanezrimaya commented Jan 29, 2024

woodruffw commented Jan 29, 2024

woodruffw commented Jan 30, 2024

vikanezrimaya commented Jan 30, 2024

vikanezrimaya commented Feb 2, 2024

woodruffw commented Mar 27, 2024

vikanezrimaya commented Mar 29, 2024

woodruffw commented Apr 3, 2024

vikanezrimaya commented Apr 5, 2024

woodruffw left a comment

Choose a reason for hiding this comment

woodruffw commented Apr 13, 2024

vikanezrimaya commented Jan 26, 2024 •

edited

Loading