Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
daed7fe
Detect the supervised package instead of defaulting to it
Aug 4, 2026
7e5e4bf
Cover the discovery and detection paths that decide the supervised scope
Aug 6, 2026
9732c4f
Abort the permission fixtures when the environment cannot deny access
Aug 6, 2026
626e685
Point the TUM default package at de.tum.cit.aet
Aug 9, 2026
6321ab7
Stop the self-tests inheriting their supervised package from a default
Aug 10, 2026
f2b6d7b
Document what the supervised-package fallback does not guarantee
Aug 10, 2026
0136141
Merge branch 'main' into fix/detect-supervised-package-without-policy
Aug 11, 2026
7186ada
Say what the derived supervised package does not establish
Aug 12, 2026
c7380ac
Read the Gradle descriptor as structure rather than as lines
Aug 12, 2026
4baf86e
Satisfy the quality gates the new scan tripped
Aug 12, 2026
18ce34c
Refuse a derived supervised scope the compiled project contradicts
Aug 13, 2026
3a44573
Say when the discovered source roots are only part of a project
Aug 17, 2026
6757b42
Permit the packages the project has, not the one above them
Aug 17, 2026
e3ebe85
Keep the descriptor reader inside the quality gate
Aug 17, 2026
75887cd
Refuse an output whose classes no scope could be checked against
Aug 17, 2026
88d8613
Put the scanner field and the compiled fixtures where they belong
Aug 17, 2026
560fb44
Describe the scanner the manual actually documents
Aug 17, 2026
20a37dc
Report a wide test-class package instead of refusing it
Aug 17, 2026
137b901
Apply the formatter and drop the imports the shared fixture freed
Aug 17, 2026
74361eb
Cover the string forms the descriptor masker had never been run against
Aug 18, 2026
543b5ab
Make a generated test ask which classes to analyse instead of carryin…
Aug 18, 2026
0274781
Assert what a generated test now asks rather than what it used to carry
Aug 18, 2026
d837178
Ask at runtime in the WALA generation too, and stop a derived fall-ba…
Aug 18, 2026
8e039eb
Make the generated inventory answerable for, not merely present
Aug 18, 2026
c7e2d53
Put back the two things removing every filter threw out
Aug 18, 2026
9308702
Say what the timeout exemption decides, and stop overstating it
Aug 19, 2026
a6bc1f9
Ask for the supervised scope where a test case is made
Aug 19, 2026
cbf88ff
Reserve only what Ares itself must not be confused with
Aug 21, 2026
171ba75
Let an empty supervised package be an empty package
Aug 21, 2026
50b0a21
Say it once, in the place a reader looks
Aug 21, 2026
c96ba7a
Spell the hand-kept copies alike
Aug 21, 2026
8ac170c
Ask a permission both questions, and keep the released signatures
Aug 21, 2026
8e82643
Refuse a sink no supervised frame can be blamed for
Aug 21, 2026
4cdba5f
Let each branch answer for itself
Aug 21, 2026
f4537fd
Say what the empty case actually does
Aug 21, 2026
3be27a0
Leave the released Creator method the one to implement
Aug 21, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 26 additions & 10 deletions docs/securitytest/TestCaseFactoryAndBuilderManual.md
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ Defines five scanning methods that auto-detect project metadata:
|---|---|---|
| `scanForBuildMode()` | `BuildMode` | Whether the project uses Maven (`pom.xml`) or Gradle (`build.gradle`) |
| `scanForTestClasses()` | `String[]` | Fully qualified names of all classes in the **test source directory** containing `@Test` or `@Property` annotations, or extending JUnit 3's `TestCase` |
| `scanForPackageName()` | `String` | The most frequently used non-reserved package declaration across all `.java` files |
| `scanForPackageName()` | `String` | The most frequently used non-reserved package: taken from the production sources, otherwise from the compiled production output, otherwise the configured default |
| `scanForMainClassInPackage()` | `String` | The class containing `public static void main(String[])` |
| `scanForTestPath()` | `Path` | The file system path to the test source directory |

Expand All @@ -495,16 +495,18 @@ Defines five scanning methods that auto-detect project metadata:
| Aspect | Detail |
|---|---|
| **Implements** | `ProjectScanner` |
| **Technique** | Regex-based source code analysis. Walks all `.java` files under the project root and applies four compiled regex patterns. |
| **Technique** | JavaParser-backed source analysis. Walks the `.java` files under the discovered source roots and reads the parsed syntax tree; the compiled output is read with ArchUnit's `ClassFileImporter` where the sources yield nothing. |

**Regex patterns:**
**What is read from the syntax tree:**

| Pattern | Matches | Used by |
| Fact | Read from | Used by |
|---|---|---|
| `CLASS_PATTERN` | `public [final\|abstract\|strictfp] class ClassName` | `extractClassName()` |
| `PACKAGE_PATTERN` | `package com.example.foo;` | `extractPackageName()` |
| `MAIN_METHOD_PATTERN` | `public static void main(String[] args)` (including varargs) | `extractMainClass()` |
| `TEST_ANNOTATION_PATTERN` | `@Test` or `@Property` | `extractTestClass()` (which additionally treats classes containing `extends TestCase` as test classes) |
| Package declaration | the compilation unit's `PackageDeclaration` | `scanForPackageName()` |
| Type declarations | the top-level `TypeDeclaration`s, nested types included | `scanForMainClassInPackage()`, `scanForTestClasses()` |
| `main` method | a `public static void main(String[])` declaration, varargs included | `scanForMainClassInPackage()` |
| Test classes | a `@Test` or `@Property` annotation, or a JUnit 3 `TestCase` supertype resolved through the imports of the file | `scanForTestClasses()` |

Resolving the supertype through the imports is why this is not a regex: `extends TestCase` names a type, and which type it names depends on what the file imported.

**Scanning pipeline:**

Expand All @@ -515,7 +517,21 @@ ProjectSourcesFinder.findProjectSourcesPath()
→ extractor.apply(content)
```

**`scanForPackageName()` algorithm:** First filters out reserved infrastructure prefixes (via `ReservedPackageGuard.reservedPrefixOf(...)`) so a student cannot flood the project with files in a trusted namespace to make it the derived enforcement scope → counts the frequency of every remaining `package` declaration across all files → returns the most common one. This heuristic works because in a typical student project, the main source package appears in the majority of files.
**`scanForPackageName()` algorithm:** Resolution runs in three steps, each reached only when the previous one finds nothing at all.

1. **Production sources.** Reserved infrastructure prefixes are filtered out first (via `ReservedPackageGuard.reservedPrefixOf(...)`), so a package inside a trusted namespace cannot become the derived enforcement scope. The frequency of every remaining `package` declaration is counted and the most common one wins. This heuristic works because in a typical student project the main source package appears in the majority of files.
2. **Compiled production output.** Only top-level classes are counted, so a package is not weighted by how many nested or anonymous classes it happens to contain; nesting is read from the class file rather than from the `$` in the binary name, which is a legal identifier character. This step covers a project whose build descriptor the source-root discovery cannot parse, because the build tool writes its output to the conventional directory the scanner reads.
3. **The configured default** (see [Section 10.3](#103-javaprogrammingexerciseprojectscanner)), with a warning naming the roots that were searched.

> **Limitation: the derived package is a heuristic, not a boundary.** Three things it does not establish.
>
> The **vote is influenceable by whoever can add files to the project**, and in an Artemis exercise that includes the student. Only reserved prefixes are filtered out; every other namespace is fair game, so enough classes under a package of the submitter's choosing make that package the derived scope, and the five resource domains are then enforced over it instead of over the assignment. Counting compiled classes in step 2 changes the unit that is counted, not who controls it.
>
> The **output directory is assumed, not read**. Step 2 looks in `target/classes` or `build/classes/java/main`, so a build that writes its output elsewhere is not followed there and the step finds nothing rather than finding the truth.
>
> **Step 3 guarantees nothing.** If the project does not contain the default package, the analysis path resolves to a directory that does not exist: no class is imported, no resource domain is enforced at runtime, and nothing fails. The warning in the log is the only signal.
>
> An exercise that needs a scope it can rely on declares its package in the security policy. The scanner is then not consulted at all, which is the only version of this that cannot be steered from the submission.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

**`scanForTestClasses()` algorithm:** Scans only the **test source directory** (see `scanForTestPath()`) and returns every class whose file contains a `@Test` / `@Property` annotation or `extends TestCase`.

Expand All @@ -532,7 +548,7 @@ ProjectSourcesFinder.findProjectSourcesPath()

| Override | Default in `JavaProjectScanner` | Override in `JavaProgrammingExerciseProjectScanner` |
|---|---|---|
| Default package | `""` (empty string) | `"de.tum.cit.ase"` |
| Default package | `""` (empty string) | `"de.tum.cit.aet"` |
Comment thread
coderabbitai[bot] marked this conversation as resolved.
| Default main class | `"Main"` | `"Main"` (unchanged) |

When the base scanner finds no package or main class, these TUM-specific defaults ensure reasonable behaviour for Artemis-hosted exercises.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,25 +131,39 @@ public aspect JavaAspectJThreadSystemAdviceDefinitions extends JavaAspectJAbstra
* Returns true when the intercepted thread creation is owned by Ares's own
* {@code @StrictTimeout} machinery ({@code TimeoutUtils.executeWithTimeout}
* submits each timed test invocation to an executor) rather than by student
* code. Walking from the top of the stack, a {@code TimeoutUtils} frame reached
* code. Walking from the top of the stack, {@code executeWithTimeout} reached
* before any restricted-package (student) frame means the timeout machinery
* created the thread (exempt); a student frame seen first means the student
* created it (still blocked). The student's test body runs on a separate worker
* thread whose stack does not contain {@code TimeoutUtils}, so a student thread
* is never exempted.
* created it (still blocked).
* <p>
* The method name matters. A student's test body does <em>not</em> run on a
* stack free of {@code TimeoutUtils}: it runs inside
* {@code TimeoutUtils.rethrowThrowableSafe} on the timeout worker, so that class
* appears below the student's own frames. While the restricted package matched,
* the student frame was always seen first and the distinction never surfaced.
* The moment it stopped matching, every thread operation under a
* {@code @StrictTimeout} was exempted instead, silently and without any failure.
* Keying the exemption to the creating method rather than to the class removes
* that dependency on a correctly scoped package.
*/
private static boolean isThreadCreationFromAresTimeout(String restrictedPackage) {
return java.lang.StackWalker.getInstance().walk(frames -> {
java.util.Iterator<java.lang.StackWalker.StackFrame> iterator = frames.iterator();
while (iterator.hasNext()) {
String className = iterator.next().getClassName();
java.lang.StackWalker.StackFrame frame = iterator.next();
String className = frame.getClassName();
if ("de.tum.cit.ase.ares.api.internal.TimeoutUtils".equals(className)) {
return Boolean.TRUE;
// Only the frame that actually creates the worker grants the exemption.
// TimeoutUtils sits on the student's stack too, as rethrowThrowableSafe, which
// is what runs the student's test body, so accepting any TimeoutUtils frame
// exempts student code the moment restrictedPackage stops matching.
return Boolean.valueOf("executeWithTimeout".equals(frame.getMethodName()));
}
// Ares's own infrastructure frames (this advice, internals) are never student
// code, even when restrictedPackage is a broad prefix that nominally covers them
// (e.g. the self-test fallback "de.tum.cit.ase"). Skipping them lets the walk
// reach the TimeoutUtils frame that legitimately owns this @StrictTimeout worker.
// (e.g. the "de.tum.cit.ase" scope Ares' own self-tests run under). Skipping
// them lets the walk reach the TimeoutUtils frame that legitimately owns this
// @StrictTimeout worker.
if (className.startsWith("de.tum.cit.ase.ares.api.")) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,20 @@ private static boolean isThreadCreationFromCommandExecution() {
* code. Without this exemption every {@code @StrictTimeout} test whose policy
* forbids thread creation fails, because Ares blocks its own timeout worker.
* <p>
* The check is precise: walking from the top of the stack, a
* {@code TimeoutUtils} frame reached <em>before</em> any restricted-package
* The check is precise: walking from the top of the stack,
* {@code executeWithTimeout} reached <em>before</em> any restricted-package
* (student) frame means the timeout machinery created the thread (exempt); a
* student frame seen first means the student created it (still blocked). The
* student's own test body runs on a separate worker thread whose stack does not
* contain {@code TimeoutUtils}, so a student thread is never exempted.
* student frame seen first means the student created it (still blocked).
* <p>
* The method name matters. A student's test body does <em>not</em> run on a
* stack free of {@code TimeoutUtils}: it runs inside
* {@code TimeoutUtils.rethrowThrowableSafe} on the timeout worker, so that
* class appears below the student's own frames. While the restricted package
* matched, the student frame was always seen first and the distinction never
* surfaced. The moment it stopped matching, every thread operation under a
* {@code @StrictTimeout} was exempted instead, silently and without any
* failure. Keying the exemption to the creating method rather than to the class
* removes that dependency on a correctly scoped package.
*
* @param restrictedPackage the configured restricted (student) package prefix
* @return {@code true} if the thread creation belongs to Ares's timeout
Expand All @@ -137,16 +145,20 @@ private static boolean isThreadCreationFromAresTimeout(@Nullable String restrict
return java.lang.StackWalker.getInstance().walk(frames -> {
java.util.Iterator<java.lang.StackWalker.StackFrame> iterator = frames.iterator();
while (iterator.hasNext()) {
String className = iterator.next().getClassName();
java.lang.StackWalker.StackFrame frame = iterator.next();
String className = frame.getClassName();
if ("de.tum.cit.ase.ares.api.internal.TimeoutUtils".equals(className)) {
return Boolean.TRUE;
// Only the frame that actually creates the worker grants the exemption.
// TimeoutUtils sits on the student's stack too, as rethrowThrowableSafe,
// which is what runs the student's test body, so accepting any TimeoutUtils
// frame exempts student code the moment restrictedPackage stops matching.
return Boolean.valueOf("executeWithTimeout".equals(frame.getMethodName()));
}
// Ares's own infrastructure frames (this advice, internals) are never student
// code, even when restrictedPackage is a broad prefix that nominally covers
// them
// (e.g. the self-test fallback "de.tum.cit.ase"). Skipping them lets the walk
// reach the TimeoutUtils frame that legitimately owns this @StrictTimeout
// worker.
// them (e.g. the "de.tum.cit.ase" scope Ares' own self-tests run under).
// Skipping them lets the walk reach the TimeoutUtils frame that legitimately
// owns this @StrictTimeout worker.
if (className.startsWith("de.tum.cit.ase.ares.api.")) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,16 @@ public abstract class TestCaseAbstractFactoryAndBuilder {
*/
@Nonnull
protected final String mainClassInPackageName;

/**
* Whether the supervised scope was derived from the project rather than pinned
* by a policy.
* <p>
* Only a derived scope is checked against the compiled output before
* enforcement. A pinned one is the instructor's statement of what is supervised
* and may deliberately cover part of the project.
*/
protected final boolean supervisedScopeWasDerived;
// </editor-fold>

// <editor-fold desc="Test lists">
Expand Down Expand Up @@ -263,6 +273,7 @@ public TestCaseAbstractFactoryAndBuilder(@Nonnull Creator creator, @Nonnull Writ
this.resourceAccesses = ResourceAccesses.createRestrictive();
this.testClasses = new ArrayList<>(Arrays.asList(projectScanner.scanForTestClasses()));
}
this.supervisedScopeWasDerived = securityPolicy == null;
// Reserved-package guard (#2): the supervised package - pinned by the policy or
// scanned from the project - must not fall under a trusted infrastructure
// prefix, or the supervised code would be trusted by name and bypass every
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,17 @@ public JavaTestCaseFactoryAndBuilder(@Nonnull JavaCreator creator, @Nonnull Java
@Nullable SecurityPolicy securityPolicy, @Nullable Path projectPath) {
super(creator, writer, executer, essentialDataReader, projectScanner, essentialPackagesPath,
essentialClassesPath, buildMode, architectureMode, aopMode, securityPolicy, projectPath);
this.javaProjectScanner = projectScanner;
}

/**
* The scanner, kept at its own type so that the coverage check can be reached.
* The inherited field is declared as the interface, which the check is
* deliberately not part of: it belongs to deriving a scope from a Java project,
* not to scanning one.
*/
@Nonnull
private final JavaProjectScanner javaProjectScanner;
// </editor-fold>
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// <editor-fold desc="Write security test cases methods">
Expand Down Expand Up @@ -126,6 +136,13 @@ public List<Path> writeTestCases(@Nonnull Path testFolderPath) {
*/
@Override
public void executeTestCases() {
if (supervisedScopeWasDerived) {
// Here rather than where the scope was derived: derivation also runs while
// test cases are being written, before anything is compiled. This is the last
// point at which nothing is armed yet and the compiled output already says
// what will run.
javaProjectScanner.requireDerivedScopeToCoverTheProject(packageName);
Comment thread
Claudia-Anthropica marked this conversation as resolved.
}
executer.executeTestCases(buildMode, architectureMode, aopMode, essentialPackages, essentialClasses,
testClasses, packageName, mainClassInPackageName,
this.architectureTestCases.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ public JavaProgrammingExerciseProjectScanner(BuildToolConfiguration buildConfigu
* declares no package. Overriding the {@code protected} default (rather than
* re-implementing {@code scanForPackageName}) lets the parent's polymorphic
* fallback pick this up.
* <p>
* This is the root package the Artemis exercise templates at TUM are generated
* with. It is deliberately not Ares' own {@code de.tum.cit.ase} namespace: that
* one identifies this library, whereas this value is a guess about the
* supervised project, and the two looking alike is what makes them easy to
* conflate.
*
* @since 2.0.0
* @author Markus Paulsen
Expand All @@ -29,7 +35,7 @@ public JavaProgrammingExerciseProjectScanner(BuildToolConfiguration buildConfigu
@Override
@Nonnull
protected String getDefaultPackage() {
return "de.tum.cit.ase";
return "de.tum.cit.aet";
}

/**
Expand Down
Loading
Loading