[MWAR-471] normalize file permissions on dependencies copied to WEB-INF/lib

[elharo]

When building on Windows with WSL sharing a disk with the local repository, dependency jars copied to WEB-INF/lib may inherit executable permissions from the local repository.

This normalizes file permissions on copied files to:

• Not executable
• Readable by everyone
• Writable only by owner

Fixes #539

[Copilot]

Pull request overview

This PR addresses MWAR-471 / #539 by ensuring files copied into the exploded webapp (notably dependencies in WEB-INF/lib) do not accidentally end up with executable permissions when building in Windows/WSL disk-sharing setups.

Changes:

• Normalize destination file permissions after copy (clear executable bit; set read-for-all, write-for-owner).
• Add a new unit test covering executable-bit normalization during copyFile(...).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/main/java/org/apache/maven/plugins/war/packaging/AbstractWarPackagingTask.java	Normalizes file permissions on copied files after FileUtils.copyFile(...).
src/test/java/org/apache/maven/plugins/war/packaging/AbstractWarPackagingTaskTest.java	Adds tests asserting copied artifacts are not executable (and intended to validate permission normalization).

Comments suppressed due to low confidence (1)

src/test/java/org/apache/maven/plugins/war/packaging/AbstractWarPackagingTaskTest.java:73

• Same cross-platform concern as the previous test: without a POSIX-permissions assumption, this can behave unexpectedly on filesystems that don't support executable bits (e.g., Windows).

        File source = new File(tempDir, "source.jar");
        assertTrue(source.createNewFile());
        source.setExecutable(false, false);
        source.setReadable(true, false);
        source.setWritable(true, true);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ascheman]

Thanks @elharo — the permission normalization is a sensible fix for #539, and good to see it covered with tests. One thing to rework in the normalization block, plus a couple of optional notes.

Side-effecting calls inside a boolean condition (root of a short-circuit bug).

if (!destination.setExecutable(false, false)
        || !destination.setReadable(true, false)
        || !destination.setWritable(true, true)) {
    context.getLog().debug("Could not normalize permissions for " + targetFilename);
}

Each set* call is both an action (mutates the file) and a status check, evaluated inside the if. Two problems: it reads as a pure condition while actually performing side effects, and — because || short-circuits — if setExecutable(false, false) returns false, setReadable and setWritable are never called. A failure in the first op silently skips the other two, which is exactly the Windows/WSL edge case this PR targets (on normal POSIX all three succeed, so the two happy-path tests don't catch it).

Make each action an explicit statement and check the outcome separately:

boolean ok = destination.setExecutable(false, false);
ok &= destination.setReadable(true, false);
ok &= destination.setWritable(true, true);
if (!ok) {
    context.getLog().debug("Could not normalize permissions for " + targetFilename);
}

Optional (non-blocking):

• The hand-written TestWarPackagingContext is a fair choice today since there's no mocking framework in the project. If you're up for it, phasing in Mockito (a common test dependency across the Maven plugins) — even as its own small PR — would let this and future tests mock(WarPackagingContext.class) and stub just the couple of methods they need, instead of maintaining a ~235-line by-hand implementation. Purely a future-improvement suggestion, not needed for this PR.
• The normalization applies to every unfiltered copied file, not only WEB-INF/lib jars — broader than #539's wording, though reasonable (webapp content generally shouldn't be executable). A one-line note that the wider scope is intentional would help.

Otherwise looks good — happy to approve once the normalization block is reworked.