[BUG] `__rich_repr__()` protocol is prone to escaping issues

[finite-state-machine]

• I've checked docs and closed issues for possible solutions.
• I can't find my issue in the FAQ.

Describe the bug

The __rich_repr__() protocol allows the following types to be yielded:

1. tuple[None, Any] - positional, safe form
2. tuple[str, Any] - keyword
3. tuple[None, Any, Any] - positional with default
4. tuple[str, Any, Any] - keyword with default
5. Any - positional, unsafe form

That the protocol accepts (5: Any) easily leads to errors when the Any in question happens to be a tuple (an escaping and injection problem).

Example

This project's own @auto decorator makes this mistake (c.f.: #4014, #4016), leading to TypeErrors later in rendering with no obvious cause or fix.

Possibilities for addressing this

One way to mitigate the issue would be to require/encourage yielding positionals in form 1 (tuple[None, Any]) rather than form 5 (Any). (This might take the form of documentation, deprecation, facilitating static type checks, opt-in runtime safety checks, any combination of these, etc.)

Static type checking

In repr.py, Result is currently defined as follows:
Result = Iterable[Union[Any, Tuple[Any], Tuple[str, Any], Tuple[str, Any, Any]]]

Since the Union includes Any, in practice, any iterator (generator function) will pass the type check.

Aside: In fact, the Result definition is incomplete/incorrect in that it doesn't spell out that forms 1 (Tuple[None, Any]) and 3 (Tuple[None, Any, Any]) are perfectly valid. I would guess this has gone unnoticed because the dangerously-broad form 5 (Any) happens to cover these cases.

It might be helpful to add:

ResultSafe = Iterable[tuple[str | None, Any], tuple[str | None, Any, Any]]

Developers who use ResultSafe rather than Result as the return annotation when writing a __rich_repr__() will benefit from warnings about most potentially-hazardous yield statements.

Runtime type checking

In the simplest case, developers would opt-in to yielding only tuples (valid ResultSafevalues, as defined above). If anything other than a tuple is yielded, it should be treated as an error. This won't help when the unescaped value actually is a tuple, but it will help developers maintain good discipline and perhaps detect some cases where the value is not always a tuple.

It might even be helpful to declare and optionally require a specific class to be used rather than tuple, so the yielded values will never be valid by coincidence. This would probably have fields keyword: str | None, value: Any, and something like default: Any | <SomeSentinel>. It might be a NamedTuple, or even just a trivial tuple subclass.

Implementations might opt-in to runtime type checks by decorator, or (though it's a bit hacky) yielding a RichReprConfig object before anything else. Aside: In either case, it might be helpful to have a single decorator for __rich_repr__()s which also accepts angular: bool = False. Some type checkers, e.g., Pyright, will complain about setting undeclared attributes on FunctionTypes, making __rich_repr__.angular = True a source of noise; a decorator to set angular would provide a graceful workaround.)

Combining with static typing

The decorator might require the decorated __rich_repr__() implementation to be annotated as returning ResultSafe. A @deprecated @overload (so the old protocol would work, but trigger a type checker warning) would be preferable, but decorator factories (as opposed to parameterless decorators) can't effectively leverage @deprecated yet, at least in pyright: microsoft/pyright#11292.

Deprecation

It may be desirable to gradually deprecate form 5 (Any), perhaps by issuing a warning when a non-tuple is yielded from __rich_repr__() or when no safer-form-only opt-in signal is found.

Documentation

In my opinion, documentation should be amended strongly discourage form 5 (Any), and to encourage use of whatever safeguards the Textual/Rich team opts to provide.

Platform

All affected. Details omitted.

[github-actions]

Thank you for your issue. Give us a little time to review it.

PS. You might want to check the FAQ if you haven't done so already.

Rich was created by Will McGugan. Consider sponsoring Will's work on Rich.

This is an automated reply, generated by FAQtory

[finite-state-machine]

There a few opportunities to improve safety and robustness in iter_rich_args() (nested within traverse(), rich/pretty.py +633 or so).

First, it would be helpful to be consistent about what is done with "unexpected" tuples (those not in form (value,), (key: str | None, value), or (key: str | None, value, default)). Sensible choices include:

• raise TypeError (recommended)
• treat these tuples as a bare (unencapsulated) value which happens to be a tuple

Since no bare tuple is handled correctly today, it's probably best to prohibit these (TypeError) rather "fixing" traverse() so they're handled as form 5 (Any, or in this case, tuple[Any, ...]) positionals.

Points of inconsistency include:

• empty tuples and tuples with len() >= 4 should be handled as above; they're currently silently discarded
• if key ∉ str | None, the entire tuple should be handled as above; today, these typically triggerTypeErrors much later, where the cause and fix are much less obvious

I encourage adding a mechanism to enable issuing warnings or TypeErrors for any/all bare positionals (including unexpected-form tuples, if that's the path chosen.)