from __future__ import annotations incompatible with strawberry.Private
#1586
Assignees
Labels
bug
Something isn't working
Comments
|
we call vs without the import still checking it out, i think we can call .resolve() to fix it? |
|
Very easy to reproduce. This should be considered a bug. import strawberry
@strawberry.type
class PrivateForward:
not_seen: "strawberry.Private[int]"
strawberry.Schema(query=PrivateForward)I'm investigating this now |
|
This behavior was scheduled to be included by default with Python 3.10 but was postponed for this reason: multiple libraries are not compatible with it. To make this work, one should add code to resolve the type from string: https://www.python.org/dev/peps/pep-0563/#resolving-type-hints-at-runtime |
from typing import get_type_hints
class A:
...
class B:
def toto() -> A:
...
print(B.toto.__annotations__)
print(get_type_hints(B.toto))Returns: from __future__ import annotations
from typing import get_type_hints
class A:
...
class B:
def toto() -> A:
...
print(B.toto.__annotations__)
print(get_type_hints(B.toto))Returns: |
We do this already :) unfortunately we do check for Private before that happens, so we need to refactor the check to make it work properly :) |
|
Examining this problem I came to the following conclusion: If one creates a schema where the from __future__ import annotations
import strawberry
@strawberry.type
class Foo:
bar: strawberry.Private[Bar]
class Bar:
passIn the above, when def from_object(self, object_type: TypeDefinition) -> GraphQLObjectType:
...
def get_graphql_fields() -> Dict[str, GraphQLField]:
graphql_fields = {}
for field in object_type.fields:
if not is_private(field.type):
field_name = self.config.name_converter.from_field(field)
graphql_fields[field_name] = self.from_field(field)
return graphql_fieldsTo avoid duplication, the checks for private fields in |
|
thanks @skilkis ! @BryceBeagle is now working on the solution, he'll update this soon |
skilkis
added a commit
to skilkis/strawberry
that referenced
this issue
Feb 25, 2022
This commit Closes strawberry-graphql#1586 by checking for private fields at schema conversion time. As per [PEP-563], in the future Python annotations will no longer be evaluated at definition time. As reported by Issue strawberry-graphql#1586, this means that `strawberry.Private` is incompatible with postponed evaluation as the check for a private field requires an evaluated type-hint annotation to work. By checking for private fields at schema conversion time, it is guaranteed that all fields that should be included in the schema are resolvable using an eval. This ensures that the current approach for defining private fields can be left intact. The current filtering for fields annotated with `strawberry.Private` in `types.type_resolver.py` are left intact to not needlessly instantiate `StrawberryField` objects when `strawberry.Private` is resolvable. Summary of Changes: - Added check for private fields at schema evaluation time - Added test to check that postponed evaluation with `strawberry.Private` functions correctly - Reduced code duplication in `schema_converter.py` [PEP-563]: https://www.python.org/dev/peps/pep-0563/
6 tasks
|
@BryceBeagle I don't want to step on your toes here. I really needed this fix for my current work and wanted to contribute to resolving this issue. Feel free to reject the PR if you want to implement a different solution! Cheers 🍺 |
|
Hey @skilkis, thanks for working on this, and sorry for the delay. Don't fear about stepping on my toes; I hadn't found a working solution to the problem yet. I'll take a look at your PR tonight 👍 Edit: tomorrow... |
skilkis
added a commit
to skilkis/strawberry
that referenced
this issue
Mar 7, 2022
This commit Closes strawberry-graphql#1586 by checking for private fields at schema conversion time. As per [PEP-563], in the future Python annotations will no longer be evaluated at definition time. As reported by Issue strawberry-graphql#1586, this means that `strawberry.Private` is incompatible with postponed evaluation as the check for a private field requires an evaluated type-hint annotation to work. By checking for private fields at schema conversion time, it is guaranteed that all fields that should be included in the schema are resolvable using an eval. This ensures that the current approach for defining private fields can be left intact. The current filtering for fields annotated with `strawberry.Private` in `types.type_resolver.py` are left intact to not needlessly instantiate `StrawberryField` objects when `strawberry.Private` is resolvable. Summary of Changes: - Added check for private fields at schema evaluation time - Added test to check that postponed evaluation with `strawberry.Private` functions correctly - Reduced code duplication in `schema_converter.py` [PEP-563]: https://www.python.org/dev/peps/pep-0563/
patrick91
pushed a commit
to skilkis/strawberry
that referenced
this issue
Mar 8, 2022
This commit Closes strawberry-graphql#1586 by checking for private fields at schema conversion time. As per [PEP-563], in the future Python annotations will no longer be evaluated at definition time. As reported by Issue strawberry-graphql#1586, this means that `strawberry.Private` is incompatible with postponed evaluation as the check for a private field requires an evaluated type-hint annotation to work. By checking for private fields at schema conversion time, it is guaranteed that all fields that should be included in the schema are resolvable using an eval. This ensures that the current approach for defining private fields can be left intact. The current filtering for fields annotated with `strawberry.Private` in `types.type_resolver.py` are left intact to not needlessly instantiate `StrawberryField` objects when `strawberry.Private` is resolvable. Summary of Changes: - Added check for private fields at schema evaluation time - Added test to check that postponed evaluation with `strawberry.Private` functions correctly - Reduced code duplication in `schema_converter.py` [PEP-563]: https://www.python.org/dev/peps/pep-0563/
patrick91
added a commit
that referenced
this issue
Mar 8, 2022
* Make `strawberry.Private` compatible with PEP-563 This commit Closes #1586 by checking for private fields at schema conversion time. As per [PEP-563], in the future Python annotations will no longer be evaluated at definition time. As reported by Issue #1586, this means that `strawberry.Private` is incompatible with postponed evaluation as the check for a private field requires an evaluated type-hint annotation to work. By checking for private fields at schema conversion time, it is guaranteed that all fields that should be included in the schema are resolvable using an eval. This ensures that the current approach for defining private fields can be left intact. The current filtering for fields annotated with `strawberry.Private` in `types.type_resolver.py` are left intact to not needlessly instantiate `StrawberryField` objects when `strawberry.Private` is resolvable. Summary of Changes: - Added check for private fields at schema evaluation time - Added test to check that postponed evaluation with `strawberry.Private` functions correctly - Reduced code duplication in `schema_converter.py` [PEP-563]: https://www.python.org/dev/peps/pep-0563/ * Add RELEASE.md * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add docstring to `_get_thunk_mapping` * Improve testing of private fields This commit mainly addresses a limitation of PEP-563 wherby annotations tagetting classes defined outside of module-level scope (i.e. a function) cannot be resolved. As a result, even at schema-conversion time, checking `is_private` is not guaranteed to evaluate to a valid type. As a result, a test is added to ensure that a `strawberry.Private` annotated field that forward-references a locally-defined class does not appear in the generated Schema. Although this is a failing test for now it demonstrates the issue to prevent a potential security vulnerability. Summary of Changes: - Check that the `notSeen` private field does not exist in the schema and that querying it returns no data. - Add failing test to show issue of classes defined outside module-scope - Move import of dataclass to the top of the file - Correct typo in the word `accessible` * Prevent unresolved types from entering the schema This commit changes `StrawberryField.type` to return a new sentinel `UNRESOLVED` instead of `None` to make sure that unresolvable types such as those defined within a local scope of a function do not appear in the schema as a `Void` field. With the new behavior a `TypeError` will be raised during schema conversion if any types are still unresolved. Most importantly, this new behavior prevents a security vulnerability where `Strawberry.Private` annotated fields would previously appear in the schema as a `Void` field; thereby, becomming executible and potentially leaking sensitive data in the resultant error message. Summary of Changes: - Return a new sentinel `UNRESOLVED` from `StrawberryField.type` instead of `None` - Update `GraphQLCoreConverter._get_thunk_mapping` to raise a `TypeError` if any unresolved types exist during schema conversion - Update failing `test_private_field_defined_outside_module_scope` to check if the correct `TypeError` is raised and to further ensure that if no error is raised, that the `Strawberry.Private` annotated field does not enter the schema. * Make `strawberry.Private` compatible with PEP-563 This commit Closes #1586 by checking for private fields at schema conversion time. As per [PEP-563], in the future Python annotations will no longer be evaluated at definition time. As reported by Issue #1586, this means that `strawberry.Private` is incompatible with postponed evaluation as the check for a private field requires an evaluated type-hint annotation to work. By checking for private fields at schema conversion time, it is guaranteed that all fields that should be included in the schema are resolvable using an eval. This ensures that the current approach for defining private fields can be left intact. The current filtering for fields annotated with `strawberry.Private` in `types.type_resolver.py` are left intact to not needlessly instantiate `StrawberryField` objects when `strawberry.Private` is resolvable. Summary of Changes: - Added check for private fields at schema evaluation time - Added test to check that postponed evaluation with `strawberry.Private` functions correctly - Reduced code duplication in `schema_converter.py` [PEP-563]: https://www.python.org/dev/peps/pep-0563/ * Create custom exception for unresolved field types * Remove dependency on sentinel package * Fix sentinel Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Patrick Arminio <patrick.arminio@gmail.com>
msobas
pushed a commit
to msobas/strawberry
that referenced
this issue
Apr 10, 2022
…#1684) * Make `strawberry.Private` compatible with PEP-563 This commit Closes strawberry-graphql#1586 by checking for private fields at schema conversion time. As per [PEP-563], in the future Python annotations will no longer be evaluated at definition time. As reported by Issue strawberry-graphql#1586, this means that `strawberry.Private` is incompatible with postponed evaluation as the check for a private field requires an evaluated type-hint annotation to work. By checking for private fields at schema conversion time, it is guaranteed that all fields that should be included in the schema are resolvable using an eval. This ensures that the current approach for defining private fields can be left intact. The current filtering for fields annotated with `strawberry.Private` in `types.type_resolver.py` are left intact to not needlessly instantiate `StrawberryField` objects when `strawberry.Private` is resolvable. Summary of Changes: - Added check for private fields at schema evaluation time - Added test to check that postponed evaluation with `strawberry.Private` functions correctly - Reduced code duplication in `schema_converter.py` [PEP-563]: https://www.python.org/dev/peps/pep-0563/ * Add RELEASE.md * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add docstring to `_get_thunk_mapping` * Improve testing of private fields This commit mainly addresses a limitation of PEP-563 wherby annotations tagetting classes defined outside of module-level scope (i.e. a function) cannot be resolved. As a result, even at schema-conversion time, checking `is_private` is not guaranteed to evaluate to a valid type. As a result, a test is added to ensure that a `strawberry.Private` annotated field that forward-references a locally-defined class does not appear in the generated Schema. Although this is a failing test for now it demonstrates the issue to prevent a potential security vulnerability. Summary of Changes: - Check that the `notSeen` private field does not exist in the schema and that querying it returns no data. - Add failing test to show issue of classes defined outside module-scope - Move import of dataclass to the top of the file - Correct typo in the word `accessible` * Prevent unresolved types from entering the schema This commit changes `StrawberryField.type` to return a new sentinel `UNRESOLVED` instead of `None` to make sure that unresolvable types such as those defined within a local scope of a function do not appear in the schema as a `Void` field. With the new behavior a `TypeError` will be raised during schema conversion if any types are still unresolved. Most importantly, this new behavior prevents a security vulnerability where `Strawberry.Private` annotated fields would previously appear in the schema as a `Void` field; thereby, becomming executible and potentially leaking sensitive data in the resultant error message. Summary of Changes: - Return a new sentinel `UNRESOLVED` from `StrawberryField.type` instead of `None` - Update `GraphQLCoreConverter._get_thunk_mapping` to raise a `TypeError` if any unresolved types exist during schema conversion - Update failing `test_private_field_defined_outside_module_scope` to check if the correct `TypeError` is raised and to further ensure that if no error is raised, that the `Strawberry.Private` annotated field does not enter the schema. * Make `strawberry.Private` compatible with PEP-563 This commit Closes strawberry-graphql#1586 by checking for private fields at schema conversion time. As per [PEP-563], in the future Python annotations will no longer be evaluated at definition time. As reported by Issue strawberry-graphql#1586, this means that `strawberry.Private` is incompatible with postponed evaluation as the check for a private field requires an evaluated type-hint annotation to work. By checking for private fields at schema conversion time, it is guaranteed that all fields that should be included in the schema are resolvable using an eval. This ensures that the current approach for defining private fields can be left intact. The current filtering for fields annotated with `strawberry.Private` in `types.type_resolver.py` are left intact to not needlessly instantiate `StrawberryField` objects when `strawberry.Private` is resolvable. Summary of Changes: - Added check for private fields at schema evaluation time - Added test to check that postponed evaluation with `strawberry.Private` functions correctly - Reduced code duplication in `schema_converter.py` [PEP-563]: https://www.python.org/dev/peps/pep-0563/ * Create custom exception for unresolved field types * Remove dependency on sentinel package * Fix sentinel Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Patrick Arminio <patrick.arminio@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Using a minor variation from the getting started tutorial (addition of
from __future__ import annotationsand the use ofstrawberry.Private) results in a traceback. The code works as expected when commenting out thefrom __future__import.Starting the built-in strawberry server results in the following traceback:
Python version:
Strawberry version:
OS: RHEL7.9
Upvote & Fund
The text was updated successfully, but these errors were encountered: