Throw for change-tracking proxy properties whose setters cannot be overridden

src/EFCore.Proxies/Properties/ProxiesStrings.resx

@@ -135,6 +135,12 @@
   <data name="NonVirtualProperty" xml:space="preserve">
     <value>Property '{1_entityType}.{0_property}' is not virtual. 'UseChangeTrackingProxies' requires all entity types to be public, unsealed, have virtual properties, and have a public or protected constructor. 'UseLazyLoadingProxies' requires only the navigation properties be virtual.</value>
   </data>
+  <data name="NonVisibleIndexerProperty" xml:space="preserve">
+    <value>The setter of the mapped indexer property on entity type '{entityType}' cannot be overridden by a proxy because it is not visible to the proxy assembly. Make it public or protected, or add '[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]' to the assembly that declares the entity type (a strong-named assembly must specify the proxy assembly's public key).</value>
+  </data>
+  <data name="NonVisibleProperty" xml:space="preserve">
+    <value>Property '{entityType}.{property}' cannot be overridden by a proxy because its {accessor} is not visible to the proxy assembly. Make it public or protected, or add '[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]' to the assembly that declares the entity type (a strong-named assembly must specify the proxy assembly's public key).</value>
+  </data>
   <data name="ProxiesNotEnabled" xml:space="preserve">
     <value>Unable to create proxy for '{entityType}' because proxies are not enabled. Call 'DbContextOptionsBuilder.UseChangeTrackingProxies' or 'DbContextOptionsBuilder.UseLazyLoadingProxies' to enable proxies.</value>
   </data>

src/EFCore.Proxies/Proxies/Internal/ProxyBindingRewriter.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Castle.DynamicProxy;
 using Microsoft.EntityFrameworkCore.Internal;
 
 namespace Microsoft.EntityFrameworkCore.Proxies.Internal;
@@ -92,6 +93,13 @@ public virtual void ProcessModelFinalizing(
                                 throw new InvalidOperationException(
                                     ProxiesStrings.NonVirtualProperty(navigationBase.Name, entityType.DisplayName()));
                             }
+
+                            if (navigationBase.PropertyInfo.SetMethod is { } setMethod
+                                && !IsVisibleToProxies(setMethod))
+                            {
+                                throw new InvalidOperationException(
+                                    ProxiesStrings.NonVisibleProperty(navigationBase.Name, entityType.DisplayName(), "setter"));
+                            }
                         }
 
                         if (_options.UseLazyLoadingProxies
@@ -113,6 +121,16 @@ public virtual void ProcessModelFinalizing(
                                         ProxiesStrings.NonVirtualProperty(navigationBase.Name, entityType.DisplayName()));
                                 }
                             }
+                            else if (!IsVisibleToProxies(navigationBase.PropertyInfo.GetMethod!))
+                            {
+                                // The proxy cannot override the getter, so the navigation would silently never be loaded.
+                                if (!_options.IgnoreNonVirtualNavigations
+                                    && navigationBase is not INavigation { ForeignKey.IsOwnership: true })
+                                {
+                                    throw new InvalidOperationException(
+                                        ProxiesStrings.NonVisibleProperty(navigationBase.Name, entityType.DisplayName(), "getter"));
+                                }
+                            }
                             else
                             {
                                 navigationBase.SetPropertyAccessMode(PropertyAccessMode.Field);
@@ -184,6 +202,11 @@ public virtual void ProcessModelFinalizing(
                                             ProxiesStrings.NonVirtualIndexerProperty(entityType.DisplayName()));
                                     }
                                 }
+                                else if (!IsVisibleToProxies(property.PropertyInfo.SetMethod!))
+                                {
+                                    throw new InvalidOperationException(
+                                        ProxiesStrings.NonVisibleIndexerProperty(entityType.DisplayName()));
+                                }
                             }
                         }
                         else
@@ -199,10 +222,25 @@ public virtual void ProcessModelFinalizing(
                                 throw new InvalidOperationException(
                                     ProxiesStrings.NonVirtualProperty(property.Name, entityType.DisplayName()));
                             }
+
+                            if (property.PropertyInfo.SetMethod is { } propertySetMethod
+                                && !IsVisibleToProxies(propertySetMethod))
+                            {
+                                throw new InvalidOperationException(
+                                    ProxiesStrings.NonVisibleProperty(property.Name, entityType.DisplayName(), "setter"));
+                            }
                         }
                     }
                 }
             }
         }
     }
+
+    // Defer to Castle so that the rules match exactly what the proxy generator can override, including
+    // [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]. Castle also requires the declaring type itself to be accessible,
+    // but an inaccessible entity type already fails with Castle's own error when a proxy is created, so only the visibility of
+    // the accessor is validated here.
+    private static bool IsVisibleToProxies(MethodInfo method)
+        => !ProxyUtil.IsAccessible(method.DeclaringType!)
+            || ProxyUtil.IsAccessible(method, out _);
 }

test/EFCore.Proxies.Tests/ChangeDetectionProxyTests.cs

@@ -26,6 +26,38 @@ public void Throws_if_non_virtual_property()
             Assert.Throws<InvalidOperationException>(() => context.Model).Message);
     }
 
+    [Fact]
+    public void Throws_if_property_setter_is_not_visible_to_proxies()
+    {
+        using var context = new ChangeContext<ChangeInternalSetPropEntity>(
+            entityBuilderAction: b => b.Property(e => e.Name));
+        Assert.Equal(
+            ProxiesStrings.NonVisibleProperty(
+                nameof(ChangeInternalSetPropEntity.Name), nameof(ChangeInternalSetPropEntity), "setter"),
+            Assert.Throws<InvalidOperationException>(() => context.Model).Message);
+    }
+
+    [Fact]
+    public void Throws_if_key_property_setter_is_not_visible_to_proxies()
+    {
+        using var context = new ChangeContext<ChangeInternalSetKeyEntity>(
+            entityBuilderAction: b => b.HasKey(e => e.Id));
+        Assert.Equal(
+            ProxiesStrings.NonVisibleProperty(
+                nameof(ChangeInternalSetKeyEntity.Id), nameof(ChangeInternalSetKeyEntity), "setter"),
+            Assert.Throws<InvalidOperationException>(() => context.Model).Message);
+    }
+
+    [Fact]
+    public void Does_not_throw_if_property_setter_is_protected()
+    {
+        using var context = new ChangeContext<ChangeProtectedSetPropEntity>(
+            entityBuilderAction: b => b.Property(e => e.Name));
+        Assert.NotNull(
+            context.Model.FindEntityType(typeof(ChangeProtectedSetPropEntity))!.FindProperty(
+                nameof(ChangeProtectedSetPropEntity.Name)));
+    }
+
     [Fact]
     public void Throws_if_non_virtual_indexer_property()
     {
@@ -325,6 +357,25 @@ public sealed class ChangeSealedEntity
         public int Id { get; set; }
     }
 
+    public class ChangeInternalSetPropEntity
+    {
+        public virtual int Id { get; set; }
+
+        public virtual string? Name { get; internal set; }
+    }
+
+    public class ChangeInternalSetKeyEntity
+    {
+        public virtual int Id { get; internal set; }
+    }
+
+    public class ChangeProtectedSetPropEntity
+    {
+        public virtual int Id { get; set; }
+
+        public virtual string? Name { get; protected set; }
+    }
+
     public class ChangeNonVirtualPropEntity
     {
         public int Id { get; set; }