Implement inbound checks (#24)

thisisalexandercook · web-flow · commit edde206f5b91 · 2026-01-30T15:36:58.000-05:00
* feat: add bridge return checks

* feat: add field read check

* feat: add unchecked method return checks

* chore: spotless
diff --git a/checker/src/main/java/io/github/eisop/runtimeframework/checker/nullness/NullnessVerifier.java b/checker/src/main/java/io/github/eisop/runtimeframework/checker/nullness/NullnessVerifier.java
@@ -10,8 +10,7 @@
 public class NullnessVerifier implements RuntimeVerifier {
 
   private static final ClassDesc VERIFIER = ClassDesc.of(NullnessRuntimeVerifier.class.getName());
-  private static final ClassDesc ATTRIBUTION_KIND =
-      ClassDesc.of(AttributionKind.class.getName());
+  private static final ClassDesc ATTRIBUTION_KIND = ClassDesc.of(AttributionKind.class.getName());
 
   private static final String METHOD_DEFAULT = "checkNotNull";
   private static final MethodTypeDesc DESC_DEFAULT =
diff --git a/checker/src/test/resources/test-cases/nullness-bridge/InheritanceBridgeTest.java b/checker/src/test/resources/test-cases/nullness-bridge/InheritanceBridgeTest.java
@@ -28,14 +28,19 @@ public static void main(String[] args) {
         test.finalAction(null);
         // cannot bridge final methods, no error here
 
-        String unsafe = test.returnAction();
+        // :: error: (Return value of inherited method returnAction must be NonNull)
+        test.returnAction();
+
+        // :: error: (Return value of inherited method returnAction must be NonNull)
         // :: error: (Local Variable Assignment (Slot 2) must be NonNull)
+        String unsafe = test.returnAction();
 
+        // :: error: (Return value of inherited method returnAction must be NonNull)
         @Nullable String again = test.returnAction();
     }
 
     @Override
     public void overrideMe(@NonNull String inputA, @Nullable String inputB) {
         System.out.println("safe version of this method" + inputA + inputB);
     }
-}
+}
diff --git a/checker/src/test/resources/test-cases/nullness-field-read/InstanceFieldRead.java b/checker/src/test/resources/test-cases/nullness-field-read/InstanceFieldRead.java
@@ -8,12 +8,22 @@ static class UncheckedLib {
         public String poison = null;
     }
 
+    public static void consume(@Nullable Object o) {}
+
     public static void main(String[] args) {
         UncheckedLib lib = new UncheckedLib();
         
-        String s = lib.poison;
+        // 1. Read without storage (Argument passing)
+        // :: error: (Read Field 'poison' must be NonNull)
+        consume(lib.poison);
+
+        // 2. Assignment
+        // :: error: (Read Field 'poison' must be NonNull)
         // :: error: (Local Variable Assignment (Slot 2) must be NonNull)
+        String s = lib.poison;
 
-	@Nullable String q = lib.poison;
+        // 3. Nullable Assignment (Still checks read)
+        // :: error: (Read Field 'poison' must be NonNull)
+        @Nullable String q = lib.poison;
     }
-}
+}
diff --git a/checker/src/test/resources/test-cases/nullness-field-read/StaticFieldRead.java b/checker/src/test/resources/test-cases/nullness-field-read/StaticFieldRead.java
@@ -1,3 +1,4 @@
+import org.checkerframework.checker.nullness.qual.Nullable;
 import io.github.eisop.runtimeframework.qual.AnnotatedFor;
 
 @AnnotatedFor("nullness")
@@ -7,8 +8,16 @@ static class UncheckedLib {
         public static String POISON = null;
     }
 
+    public static void consume(@Nullable Object o) {}
+
     public static void main(String[] args) {
-        String s = UncheckedLib.POISON;
+        // 1. Read without storage
+        // :: error: (Read Field 'POISON' must be NonNull)
+        consume(UncheckedLib.POISON);
+
+        // 2. Assignment
+        // :: error: (Read Field 'POISON' must be NonNull)
         // :: error: (Local Variable Assignment (Slot 1) must be NonNull)
+        String s = UncheckedLib.POISON;
     }
-}
+}
diff --git a/checker/src/test/resources/test-cases/nullness-invoke/InstanceBoundary.java b/checker/src/test/resources/test-cases/nullness-invoke/InstanceBoundary.java
@@ -13,10 +13,11 @@ public static void main(String[] args) {
         UncheckedLib lib = new UncheckedLib();
         
         String s = lib.getNull();
+	// :: error: (Return value of getNull (Boundary) must be NonNull)
 	// :: error: (Local Variable Assignment (Slot 2) must be NonNull)
 
 	lib.getNull();
-	// currently no explicit check on the return if its not stored 
+	// :: error: (Return value of getNull (Boundary) must be NonNull)
 
     }
 }
diff --git a/checker/src/test/resources/test-cases/nullness-invoke/NullableBoundary.java b/checker/src/test/resources/test-cases/nullness-invoke/NullableBoundary.java
@@ -12,5 +12,6 @@ public static String getNull() {
 
     public static void main(String[] args) {
         @Nullable String s = UncheckedLib.getNull();
+        // :: error: (Return value of getNull (Boundary) must be NonNull)
     }
 }
diff --git a/checker/src/test/resources/test-cases/nullness-invoke/StaticBoundary.java b/checker/src/test/resources/test-cases/nullness-invoke/StaticBoundary.java
@@ -12,6 +12,7 @@ public static String getNull() {
 
     public static void main(String[] args) {
         String s = UncheckedLib.getNull();
+	// :: error: (Return value of getNull (Boundary) must be NonNull)
 	// :: error: (Local Variable Assignment (Slot 1) must be NonNull)
     }
 
diff --git a/checker/src/test/resources/test-cases/nullness-parameter/FieldArgument.java b/checker/src/test/resources/test-cases/nullness-parameter/FieldArgument.java
@@ -10,14 +10,17 @@ static class UncheckedLib {
     }
 
     public static void main(String[] args) {
+        // :: error: (Read Field 'POISON' must be NonNull)
+        // :: error: (Parameter 0 must be NonNull)
         consume(UncheckedLib.POISON);
-	nullableConsume(UncheckedLib.POISON);
+
+        // :: error: (Read Field 'POISON' must be NonNull)
+	    nullableConsume(UncheckedLib.POISON);
     }
 
     public static void consume(@NonNull String arg) {
-	// :: error: (Parameter 0 must be NonNull)
     }
     
     public static void nullableConsume(@Nullable String arg) {
     }
-}
+}
diff --git a/framework/src/main/java/io/github/eisop/runtimeframework/core/AnnotationInstrumenter.java b/framework/src/main/java/io/github/eisop/runtimeframework/core/AnnotationInstrumenter.java
@@ -154,8 +154,14 @@ protected void generateUncheckedReturnCheck(
 
   @Override
   protected void generateMethodCallCheck(CodeBuilder b, InvokeInstruction invoke) {
-    // empty for now, only need to generate checks when a method call is stored somehwhere
+    RuntimeVerifier target =
+        policy.getBoundaryCallCheck(invoke.owner().asInternalName(), invoke.typeSymbol());
 
+    if (target != null) {
+      b.dup();
+      target.generateCheck(
+          b, TypeKind.REFERENCE, "Return value of " + invoke.name().stringValue() + " (Boundary)");
+    }
   }
 
   @Override
@@ -225,6 +231,16 @@ private void emitBridge(ClassBuilder builder, ParentMethod parentMethod) {
                     ClassDesc.of(
                         parentMethod.owner().thisClass().asInternalName().replace('/', '.'));
                 codeBuilder.invokespecial(parentDesc, methodName, desc);
+
+                RuntimeVerifier returnTarget = policy.getBridgeReturnCheck(parentMethod);
+                if (returnTarget != null) {
+                  codeBuilder.dup();
+                  returnTarget.generateCheck(
+                      codeBuilder,
+                      TypeKind.REFERENCE,
+                      "Return value of inherited method " + methodName);
+                }
+
                 returnResult(
                     codeBuilder, ClassDesc.ofDescriptor(desc.returnType().descriptorString()));
               });
diff --git a/framework/src/main/java/io/github/eisop/runtimeframework/core/RuntimeInstrumenter.java b/framework/src/main/java/io/github/eisop/runtimeframework/core/RuntimeInstrumenter.java
@@ -66,13 +66,7 @@ public void accept(ClassBuilder classBuilder, ClassElement classElement) {
                             } else if (isFieldRead(fInst)) {
                               codeBuilder.with(element);
                               if (isCheckedScope) {
-                                // generateFieldReadCheck(codeBuilder, fInst, classModel);
-                                // Currently disabling field read checks as the GETFIELD
-                                // and GETSTATIC instructions are not actually dangerous
-                                // on their own. Its when we STORE a field we read from
-                                // that an issue could arise
-                                // TODO: consider method of turning on and off different
-                                // boundary sites
+                                generateFieldReadCheck(codeBuilder, fInst, classModel);
                               }
                             }
                           } else if (element instanceof ReturnInstruction rInst) {
diff --git a/framework/src/main/java/io/github/eisop/runtimeframework/policy/EnforcementPolicy.java b/framework/src/main/java/io/github/eisop/runtimeframework/policy/EnforcementPolicy.java
@@ -44,6 +44,11 @@ default RuntimeVerifier getBoundaryFieldWriteCheck(
   /** For a bridge we are generating, what check applies to this parameter? */
   RuntimeVerifier getBridgeParameterCheck(ParentMethod parentMethod, int paramIndex);
 
+  /** For a bridge we are generating, what check applies to the return value? */
+  default RuntimeVerifier getBridgeReturnCheck(ParentMethod parentMethod) {
+    return null;
+  }
+
   /** Should we check an value being stored into an array? */
   RuntimeVerifier getArrayStoreCheck(TypeKind componentType);
 
diff --git a/framework/src/main/java/io/github/eisop/runtimeframework/policy/StandardEnforcementPolicy.java b/framework/src/main/java/io/github/eisop/runtimeframework/policy/StandardEnforcementPolicy.java
@@ -5,9 +5,8 @@
 import io.github.eisop.runtimeframework.core.ValidationKind;
 import io.github.eisop.runtimeframework.filter.ClassInfo;
 import io.github.eisop.runtimeframework.filter.Filter;
-import io.github.eisop.runtimeframework.policy.EnforcementPolicy;
-import io.github.eisop.runtimeframework.runtime.AttributionKind;
 import io.github.eisop.runtimeframework.resolution.ParentMethod;
+import io.github.eisop.runtimeframework.runtime.AttributionKind;
 import java.lang.classfile.Annotation;
 import java.lang.classfile.Attributes;
 import java.lang.classfile.FieldModel;
@@ -146,6 +145,7 @@ public boolean shouldGenerateBridge(ParentMethod parentMethod) {
     // MethodTypeDesc param parsing
     var paramTypes = method.methodTypeSymbol().parameterList();
 
+    // 1. Check Parameters
     for (int i = 0; i < paramTypes.size(); i++) {
       boolean explicitNoop = false;
       boolean explicitEnforce = false;
@@ -172,6 +172,34 @@ public boolean shouldGenerateBridge(ParentMethod parentMethod) {
         }
       }
     }
+
+    // 2. Check Return Type
+    TypeKind returnType = TypeKind.from(method.methodTypeSymbol().returnType());
+    if (returnType == TypeKind.REFERENCE) {
+      boolean explicitNoop = false;
+      boolean explicitEnforce = false;
+
+      List<Annotation> annos = getMethodReturnAnnotations(method);
+
+      for (Annotation anno : annos) {
+        String desc = anno.classSymbol().descriptorString();
+        TypeSystemConfiguration.ConfigEntry entry = configuration.find(desc);
+        if (entry != null) {
+          if (entry.kind() == ValidationKind.ENFORCE) explicitEnforce = true;
+          if (entry.kind() == ValidationKind.NOOP) explicitNoop = true;
+        }
+      }
+
+      if (explicitEnforce) return true;
+
+      if (!explicitNoop) {
+        TypeSystemConfiguration.ConfigEntry defaultEntry = configuration.getDefault();
+        if (defaultEntry != null && defaultEntry.kind() == ValidationKind.ENFORCE) {
+          return true;
+        }
+      }
+    }
+
     return false;
   }
 
@@ -208,6 +236,33 @@ public RuntimeVerifier getBridgeParameterCheck(ParentMethod parentMethod, int pa
     return null;
   }
 
+  @Override
+  public RuntimeVerifier getBridgeReturnCheck(ParentMethod parentMethod) {
+    MethodModel method = parentMethod.method();
+    TypeKind returnType = TypeKind.from(method.methodTypeSymbol().returnType());
+    if (returnType != TypeKind.REFERENCE) return null;
+
+    List<Annotation> annos = getMethodReturnAnnotations(method);
+
+    RuntimeVerifier verifier = resolveVerifier(annos);
+    if (verifier != null) return verifier.withAttribution(AttributionKind.CALLER);
+
+    boolean isExplicitNoop = false;
+    for (Annotation a : annos) {
+      TypeSystemConfiguration.ConfigEntry entry =
+          configuration.find(a.classSymbol().descriptorString());
+      if (entry != null && entry.kind() == ValidationKind.NOOP) isExplicitNoop = true;
+    }
+
+    if (!isExplicitNoop) {
+      TypeSystemConfiguration.ConfigEntry defaultEntry = configuration.getDefault();
+      if (defaultEntry != null && defaultEntry.kind() == ValidationKind.ENFORCE) {
+        return defaultEntry.verifier().withAttribution(AttributionKind.CALLER);
+      }
+    }
+    return null;
+  }
+
   // --- Parsing Helpers ---
   private List<Annotation> getMethodParamAnnotations(MethodModel method, int paramIndex) {
     List<Annotation> result = new ArrayList<>();
@@ -230,6 +285,24 @@ private List<Annotation> getMethodParamAnnotations(MethodModel method, int param
     return result;
   }
 
+  private List<Annotation> getMethodReturnAnnotations(MethodModel method) {
+    List<Annotation> result = new ArrayList<>();
+    method
+        .findAttribute(Attributes.runtimeVisibleAnnotations())
+        .ifPresent(attr -> result.addAll(attr.annotations()));
+    method
+        .findAttribute(Attributes.runtimeVisibleTypeAnnotations())
+        .ifPresent(
+            attr -> {
+              for (TypeAnnotation ta : attr.annotations()) {
+                if (ta.targetInfo().targetType() == TypeAnnotation.TargetType.METHOD_RETURN) {
+                  result.add(ta.annotation());
+                }
+              }
+            });
+    return result;
+  }
+
   private List<Annotation> getFieldAnnotations(FieldModel field) {
     List<Annotation> result = new ArrayList<>();
     field

Original file line number	Diff line number	Diff line change
`@@ -13,10 +13,11 @@ public static void main(String[] args) {`
`13`	`13`	`UncheckedLib lib = new UncheckedLib();`
`14`	`14`
`15`	`15`	`String s = lib.getNull();`
	`16`	`+ // :: error: (Return value of getNull (Boundary) must be NonNull)`
`16`	`17`	`// :: error: (Local Variable Assignment (Slot 2) must be NonNull)`
`17`	`18`
`18`	`19`	`lib.getNull();`
`19`		`- // currently no explicit check on the return if its not stored`
	`20`	`+ // :: error: (Return value of getNull (Boundary) must be NonNull)`
`20`	`21`
`21`	`22`	`}`
`22`	`23`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,5 +12,6 @@ public static String getNull() {`
`12`	`12`
`13`	`13`	`public static void main(String[] args) {`
`14`	`14`	`@Nullable String s = UncheckedLib.getNull();`
	`15`	`+ // :: error: (Return value of getNull (Boundary) must be NonNull)`
`15`	`16`	`}`
`16`	`17`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ public static String getNull() {`
`12`	`12`
`13`	`13`	`public static void main(String[] args) {`
`14`	`14`	`String s = UncheckedLib.getNull();`
	`15`	`+ // :: error: (Return value of getNull (Boundary) must be NonNull)`
`15`	`16`	`// :: error: (Local Variable Assignment (Slot 1) must be NonNull)`
`16`	`17`	`}`
`17`	`18`
Original file line number	Diff line number	Diff line change
`@@ -10,14 +10,17 @@ static class UncheckedLib {`
`10`	`10`	`}`
`11`	`11`
`12`	`12`	`public static void main(String[] args) {`
	`13`	`+ // :: error: (Read Field 'POISON' must be NonNull)`
	`14`	`+ // :: error: (Parameter 0 must be NonNull)`
`13`	`15`	`consume(UncheckedLib.POISON);`
`14`		`- nullableConsume(UncheckedLib.POISON);`
	`16`	`+`
	`17`	`+ // :: error: (Read Field 'POISON' must be NonNull)`
	`18`	`+ nullableConsume(UncheckedLib.POISON);`
`15`	`19`	`}`
`16`	`20`
`17`	`21`	`public static void consume(@NonNull String arg) {`
`18`		`- // :: error: (Parameter 0 must be NonNull)`
`19`	`22`	`}`
`20`	`23`
`21`	`24`	`public static void nullableConsume(@Nullable String arg) {`
`22`	`25`	`}`
`23`		`-}`
	`26`	`+}`