Skip to content

Conversation

@enisdenjo
Copy link
Member

@enisdenjo enisdenjo commented Oct 28, 2025

Causing issues with graphql-hive/gateway#1629 due to malformed queries for query planning.

The generic-auth plugin leaves document nodes broken after validation and redaction of unauthenticated fields. Given a schema:

type Query {
  public: String
  person: Person
}
interface Person {
  name: String
}
type Admin implements Person @authenticated {
  name: String
  email: String
}

the following user query:

{
  public
  person {
    name
    ... on Admin {
      email
    }
  }
}

will be incorrectly redacted to:

{
  public
  person {
    name
-   ... on Admin
  }
}

secondly, the following query:

{
  public
  person {
    ... on Admin {
      email
    }
  }
}

will be incorrectly redacted to:

{
  public
- person {
-   ... on Admin
- }
}

finally, the following query:

{
  public
  person {
    ...A
  }
}
fragment A on Admin {
  email
}

will be incorrectly redacted to:

query {
  public
- person {
-   ...A
- }
}
- fragment A on Admin 

TODO

While working on the task, I noticed more issues with the generic-plugin itself (was broken without the fixes too) see skipped tests in use-generic-auth.spec.ts.

@enisdenjo enisdenjo requested review from ardatan and n1ru4l October 28, 2025 15:37
@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2025

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@envelop/extended-validation 6.1.0-alpha-20251029092940-df37e7855bddbc733ddcb2d790e30e44c5cb0487 npm ↗︎ unpkg ↗︎
@envelop/generic-auth 10.0.1-alpha-20251029092940-df37e7855bddbc733ddcb2d790e30e44c5cb0487 npm ↗︎ unpkg ↗︎

@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2025

💻 Website Preview

The latest changes are available as preview in: https://d67c7a0b.envelop.pages.dev

Copy link
Member

@ardatan ardatan left a comment

Choose a reason for hiding this comment

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

Fixed the failing skipped tests, and enabled them

@enisdenjo enisdenjo merged commit 62a8915 into main Oct 29, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants