Added confidential checker JDK annotations#224
Added confidential checker JDK annotations#224iywang2016 wants to merge 49 commits intotypetools:masterfrom
Conversation
# Conflicts: # src/java.base/share/classes/java/lang/Object.java # src/java.base/share/classes/org/checkerframework/checker/confidential/qual/BottomConfidential.java # src/java.base/share/classes/org/checkerframework/checker/confidential/qual/Confidential.java # src/java.base/share/classes/org/checkerframework/checker/confidential/qual/NonConfidential.java # src/java.base/share/classes/org/checkerframework/checker/confidential/qual/PolyConfidential.java # src/java.base/share/classes/org/checkerframework/checker/confidential/qual/UnknownConfidential.java # src/java.base/share/classes/org/checkerframework/checker/sqlquotes/qual/SqlEvenQuotes.java # src/java.base/share/classes/org/checkerframework/checker/sqlquotes/qual/SqlOddQuotes.java # src/java.base/share/classes/org/checkerframework/checker/sqlquotes/qual/SqlQuotesBottom.java # src/java.base/share/classes/org/checkerframework/checker/sqlquotes/qual/SqlQuotesUnknown.java
📝 WalkthroughWalkthroughThe change adds a Confidentiality checker to the Checker Framework: new qualifier annotations ( 🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/java.base/share/classes/java/lang/Exception.java`:
- Line 28: Subclasses of java.lang.Exception (notably IOException and
ReflectiveOperationException) must mirror the new `@PolyConfidential` constructor
signatures and module annotations: update each subclass's constructors that
accept message/cause (e.g., IOException(String message), IOException(String
message, Throwable cause), ReflectiveOperationException(...) and similar in
other Exception subclasses) to annotate the message and cause parameters with
`@PolyConfidential`, and add "confidential" to their `@AnnotatedFor` declarations so
they satisfy the Exception contract; ensure every super(...) call remains
compatible with the annotated types.
In `@src/java.base/share/classes/java/util/Map.java`:
- Around line 274-275: The implementations of Map.get() are missing the
`@PolyConfidential` annotation on their receiver which violates the interface
contract declared by Map.get(`@PolyConfidential` ... this) and breaks type
checking. For each implementing class (HashMap, LinkedHashMap, AbstractMap,
IdentityHashMap, WeakHashMap, TreeMap, EnumMap, ConcurrentHashMap) update the
get(...) method signature to include the `@PolyConfidential` annotation on the
receiver (matching Map.get's receiver annotation style, e.g., add
`@PolyConfidential` to the this parameter alongside existing
`@GuardSatisfied/`@UnknownSignedness annotations) and add any required imports so
the method signatures match the interface contract.
In `@src/java.logging/share/classes/java/util/logging/Handler.java`:
- Line 29: Add the `@NonConfidential` annotation to each override of the
publish(LogRecord record) method in the listed Handler subclasses so their
signatures match the annotated parent: update FileHandler.publish(LogRecord
record), SocketHandler.publish(LogRecord record),
StreamHandler.publish(LogRecord record), MemoryHandler.publish(LogRecord
record), ConsoleHandler.publish(LogRecord record), and
sun.rmi.runtime.Log.publish(LogRecord record) to include `@NonConfidential` on the
method declaration.
In `@src/java.logging/share/classes/java/util/logging/Logger.java`:
- Line 28: Remove the duplicate import of
org.checkerframework.checker.confidential.qual.NonConfidential in Logger.java:
locate the import declarations at the top of the file (related to class Logger)
and delete the redundant second occurrence so only a single import of
`@NonConfidential` remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88f7bb0d-c5ca-4cdc-928f-4a2404844bb2
📒 Files selected for processing (18)
src/java.base/share/classes/java/io/PrintStream.javasrc/java.base/share/classes/java/lang/Exception.javasrc/java.base/share/classes/java/lang/Object.javasrc/java.base/share/classes/java/lang/RuntimeException.javasrc/java.base/share/classes/java/lang/String.javasrc/java.base/share/classes/java/lang/Throwable.javasrc/java.base/share/classes/java/util/Map.javasrc/java.base/share/classes/java/util/Optional.javasrc/java.base/share/classes/java/util/stream/Stream.javasrc/java.base/share/classes/module-info.javasrc/java.base/share/classes/org/checkerframework/checker/confidential/qual/BottomConfidential.javasrc/java.base/share/classes/org/checkerframework/checker/confidential/qual/Confidential.javasrc/java.base/share/classes/org/checkerframework/checker/confidential/qual/NonConfidential.javasrc/java.base/share/classes/org/checkerframework/checker/confidential/qual/PolyConfidential.javasrc/java.base/share/classes/org/checkerframework/checker/confidential/qual/UnknownConfidential.javasrc/java.logging/share/classes/java/util/logging/Formatter.javasrc/java.logging/share/classes/java/util/logging/Handler.javasrc/java.logging/share/classes/java/util/logging/Logger.java
👮 Files not reviewed due to content moderation or server errors (5)
- src/java.logging/share/classes/java/util/logging/Formatter.java
- src/java.base/share/classes/org/checkerframework/checker/confidential/qual/BottomConfidential.java
- src/java.base/share/classes/java/lang/Object.java
- src/java.base/share/classes/java/lang/String.java
- src/java.base/share/classes/java/io/PrintStream.java
|
|
||
| package java.lang; | ||
|
|
||
| import org.checkerframework.checker.confidential.qual.PolyConfidential; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Exception subclasses with constructors taking String message that lack `@PolyConfidential`
rg -l "extends Exception" --type=java | xargs -I{} sh -c 'echo "=== {} ===" && rg -n "public.*\(.*String message" {} | head -5'Repository: typetools/jdk
Length of output: 36660
🏁 Script executed:
# Check Exception.java content
cat -n src/java.base/share/classes/java/lang/Exception.java | head -150Repository: typetools/jdk
Length of output: 6726
🏁 Script executed:
# Check IOException to see if it extends Exception and its constructor signatures
cat -n src/java.base/share/classes/java/io/IOException.java | head -100Repository: typetools/jdk
Length of output: 4428
🏁 Script executed:
# Check ReflectiveOperationException as well
cat -n src/java.base/share/classes/java/lang/ReflectiveOperationException.javaRepository: typetools/jdk
Length of output: 4257
LGTM! @PolyConfidential annotations properly added to Exception constructors.
The annotations enable confidentiality tracking through the exception hierarchy. The @PolyConfidential qualifier on message and cause parameters allows the checker to track whether exceptions contain confidential data.
However, IOException (which extends Exception) and other subclasses lack corresponding @PolyConfidential annotations on their constructors. When IOException calls super(message), it passes a plain String where Exception expects @PolyConfidential String, breaking the type contract. Update IOException, ReflectiveOperationException, and other Exception subclasses to include @PolyConfidential annotations and add "confidential" to their @AnnotatedFor declarations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/java.base/share/classes/java/lang/Exception.java` at line 28, Subclasses
of java.lang.Exception (notably IOException and ReflectiveOperationException)
must mirror the new `@PolyConfidential` constructor signatures and module
annotations: update each subclass's constructors that accept message/cause
(e.g., IOException(String message), IOException(String message, Throwable
cause), ReflectiveOperationException(...) and similar in other Exception
subclasses) to annotate the message and cause parameters with `@PolyConfidential`,
and add "confidential" to their `@AnnotatedFor` declarations so they satisfy the
Exception contract; ensure every super(...) call remains compatible with the
annotated types.
| @Pure | ||
| @Nullable V get(@GuardSatisfied Map<K, V> this, @UnknownSignedness @GuardSatisfied Object key); | ||
| @Nullable V get(@GuardSatisfied @PolyConfidential Map<K, V> this, @UnknownSignedness @GuardSatisfied Object key); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if Map implementations have `@PolyConfidential` on their get() receivers
# Expected: Find whether implementations need to be updated
# Search for get() method signatures in Map implementations
rg -n --type=java -B2 -A2 'public.*\bV\b\s+get\s*\(' | rg -A4 -B4 'HashMap|ConcurrentHashMap|AbstractMap|LinkedHashMap|TreeMap|EnumMap|IdentityHashMap|WeakHashMap'Repository: typetools/jdk
Length of output: 13632
Add @PolyConfidential annotation to the receiver of get() methods in all Map implementations.
The Map.get() interface declares @PolyConfidential on its receiver, but all implementing classes (HashMap, LinkedHashMap, AbstractMap, IdentityHashMap, WeakHashMap, TreeMap, EnumMap, ConcurrentHashMap) omit this annotation on their receivers. This inconsistency will cause type-checking failures since implementations do not satisfy the receiver type annotation contract established by the interface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/java.base/share/classes/java/util/Map.java` around lines 274 - 275, The
implementations of Map.get() are missing the `@PolyConfidential` annotation on
their receiver which violates the interface contract declared by
Map.get(`@PolyConfidential` ... this) and breaks type checking. For each
implementing class (HashMap, LinkedHashMap, AbstractMap, IdentityHashMap,
WeakHashMap, TreeMap, EnumMap, ConcurrentHashMap) update the get(...) method
signature to include the `@PolyConfidential` annotation on the receiver (matching
Map.get's receiver annotation style, e.g., add `@PolyConfidential` to the this
parameter alongside existing `@GuardSatisfied/`@UnknownSignedness annotations) and
add any required imports so the method signatures match the interface contract.
|
|
||
| package java.util.logging; | ||
|
|
||
| import org.checkerframework.checker.confidential.qual.NonConfidential; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all Handler subclasses with publish methods that might need `@NonConfidential`
rg -n "public void publish\(LogRecord" --type=java -C2 | grep -v "@NonConfidential"Repository: typetools/jdk
Length of output: 23801
Handler subclasses must also include @NonConfidential annotation
The Handler.java changes are correct. The @NonConfidential annotation on Handler.publish(LogRecord record) properly enforces that log records must not contain confidential data.
However, the following Handler subclasses override publish() without this annotation and will trigger type-checker warnings:
- FileHandler.java:778
- SocketHandler.java:192
- StreamHandler.java:221
- MemoryHandler.java:181
- ConsoleHandler.java:94
- sun.rmi.runtime.Log.java:287
These subclass implementations must include @NonConfidential to match the parent signature.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/java.logging/share/classes/java/util/logging/Handler.java` at line 29,
Add the `@NonConfidential` annotation to each override of the publish(LogRecord
record) method in the listed Handler subclasses so their signatures match the
annotated parent: update FileHandler.publish(LogRecord record),
SocketHandler.publish(LogRecord record), StreamHandler.publish(LogRecord
record), MemoryHandler.publish(LogRecord record),
ConsoleHandler.publish(LogRecord record), and
sun.rmi.runtime.Log.publish(LogRecord record) to include `@NonConfidential` on the
method declaration.
|
|
||
| package java.util.logging; | ||
|
|
||
| import org.checkerframework.checker.confidential.qual.NonConfidential; |
There was a problem hiding this comment.
Remove duplicate import of @NonConfidential.
The import org.checkerframework.checker.confidential.qual.NonConfidential appears twice:
- Line 28
- Line 38
This is redundant and will cause a compiler warning.
🔧 Proposed fix
import org.checkerframework.checker.confidential.qual.NonConfidential;
import org.checkerframework.checker.interning.qual.Interned;
import org.checkerframework.checker.interning.qual.UsesObjectEquals;
import org.checkerframework.checker.lock.qual.GuardSatisfied;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.checker.signature.qual.BinaryName;
import org.checkerframework.dataflow.qual.Pure;
import org.checkerframework.dataflow.qual.SideEffectFree;
import org.checkerframework.framework.qual.AnnotatedFor;
import org.checkerframework.framework.qual.CFComment;
-import org.checkerframework.checker.confidential.qual.NonConfidential;Also applies to: 38-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/java.logging/share/classes/java/util/logging/Logger.java` at line 28,
Remove the duplicate import of
org.checkerframework.checker.confidential.qual.NonConfidential in Logger.java:
locate the import declarations at the top of the file (related to class Logger)
and delete the redundant second occurrence so only a single import of
`@NonConfidential` remains.
2 participants
Associated with checker framework pull request #6981