Skip to content

Source generator surfaces internal/protected internal property accessors by default; reflection provider does not (provider parity break) #455

Description

@eiriktsarpalis

Summary

The source generator and the reflection provider produce different property projections for the same type. For a public property with a non-public accessor — e.g. public string Secret { get; internal set; } — defined in the same compilation as [GenerateShape]/[GenerateShapeFor], the source generator surfaces the internal/protected internal accessor (IPropertyShape.HasSetter == true), whereas the reflection provider excludes it (HasSetter == false).

This breaks the "same shape regardless of provider" guarantee and, for consumers such as deserializers/mappers, silently expands the mutation/disclosure surface of a type depending on which provider is used.

Repro

[GenerateShape]
public partial class Model
{
    public string Secret { get; internal set; }
}
  • Reflection provider: surfaced read-onlyHasGetter == true, HasSetter == false.
  • Source generator (same assembly): surfaced read-writeHasGetter == true, HasSetter == true.

Affects internal and protected internal accessors on otherwise-public properties, for types in the current compilation (or a referenced assembly with [InternalsVisibleTo]).

Root cause

The generator constructs the parser with the whole compilation assembly as the accessibility scope:

src/PolyType.SourceGenerator/Parser/Parser.cs:46

Parser parser = new(knownSymbols.Compilation.Assembly, typeShapeExtensions, knownSymbols, cancellationToken);

so IsAccessibleSymbol(s)Compilation.IsSymbolAccessibleWithin(s, <assembly>), which is true for any internal member declared in that assembly.

The default property inclusion (used when there is no [PropertyShape] opt-in) gates the property on DeclaredAccessibility == Public, then keys accessor inclusion on accessibility, not publicity:

src/PolyType.Roslyn/ModelGenerator/TypeDataModelGenerator.Object.cs:116-132

if (property.DeclaredAccessibility is not Accessibility.Public)
{
    includeGetter = includeSetter = false;
    return false;
}

property = property.GetBaseProperty();
includeGetter = property.GetMethod is { } getter && IsAccessibleSymbol(getter);  // accessibility, not publicity
includeSetter = property.SetMethod is { } setter && IsAccessibleSymbol(setter);
return true;

For public … { get; internal set; } the property's DeclaredAccessibility is Public (accessors can only restrict it), so it passes the gate; the internal setter is accessible from assembly scope ⇒ includeSetter == true. This flows through to the emitted shape:

src/PolyType.SourceGenerator/Parser/Parser.ModelMapper.cs:417-440 (EmitGetter/EmitSetterIPropertyShape.HasGetter/HasSetter).

The reflection provider instead gates on publicity:

src/PolyType/ReflectionProvider/ReflectionPropertyShape.cs:44-45

HasGetter = p.CanRead && (shapeInfo.IncludeNonPublicAccessors || p.GetMethod!.IsPublic);
HasSetter = p.CanWrite && (shapeInfo.IncludeNonPublicAccessors || p.SetMethod!.IsPublic) && !p.IsInitOnly();

IncludeNonPublicAccessors defaults to false and is only set true via [PropertyShape] opt-in (ReflectionObjectTypeShape.cs:423), so the internal setter is not surfaced by default.

The inclusion method's own doc remark already promises the reflection behavior, which the implementation contradicts:

src/PolyType.Roslyn/ModelGenerator/TypeDataModelGenerator.Object.cs:115

/// <remarks>Defaults to including public getters and setters only.</remarks>

Expected

Both providers should, by default, surface public accessors only, with non-public accessors included only on explicit [PropertyShape] opt-in. The reflection provider's public-only default is the correct baseline, and it matches the documented contract above.

Impact

  • Provider parity break: the same type has different round-trip / mutation / disclosure surfaces depending on the provider.
  • Security relevance: an internal/protected internal setter — intended for assembly-internal mutation — becomes writable from untrusted input under source-gen but not under reflection. A data-contract / security review performed against one provider does not hold for the other.

Scope / bounding (verified)

  • Properties only. Fields do not diverge — the default IncludeField requires the field itself to be Public (TypeDataModelGenerator.Object.cs:101-109), so internal fields are excluded under both providers.
  • Fully-internal properties do not diverge (excluded by the DeclaredAccessibility == Public gate).
  • private set / protected set do not diverge (not accessible from assembly scope).
  • Only internal / protected internal accessors on otherwise-public properties are affected.
  • The [PropertyShape] opt-in path (Parser.cs:224-227) correctly includes non-public accessors and should be preserved — only the no-attribute default path is wrong.

Suggested fix

Gate default accessor inclusion on publicity to match the reflection baseline and the existing doc remark, in TypeDataModelGenerator.IncludeProperty:

property = property.GetBaseProperty();
includeGetter = property.GetMethod is { DeclaredAccessibility: Accessibility.Public };
includeSetter = property.SetMethod is { DeclaredAccessibility: Accessibility.Public };
return true;

Leave IsGetterAccessible/IsSetterAccessible (which legitimately track accessibility for UnsafeAccessor vs. direct-access codegen) unchanged. Note that PolyType.Roslyn is also consumed by third-party generators, so this base-default change affects them too — but it aligns the implementation with the method's documented "public getters and setters only" contract. Alternatively the gating could be localized to the PolyType source generator if the base default is intended to remain accessibility-based.

A regression test asserting HasGetter/HasSetter parity across providers for public T { get; internal set; } / { get; protected internal set; } (and the [PropertyShape] opt-in still surfacing them) would lock this down.


Found during a threat-model review of the core programming model and the two built-in shape providers.

Metadata

Metadata

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions