Skip to content

Conversation

alexarchambault
Copy link
Collaborator

This PR makes SonatypeCentralPublishModule refuse credentials passed via --username and --password if it detects it's running on a CI. Using these arguments poses security issues: the Mill's prompt tries to print them back, like in

[4032/4037] =============== mill.scalalib.SonatypeC...N/lEdX9diiWxHsua8VLfDzw =============== 480s

(prompt obtained by running a command like ./mill -i mill.scalalib.SonatypeCentralPublishModule/ --username … --password …, containing parts of a now revoked token)

These secrets might be truncated in particular, which can make parts of these secrets slip through output secret detection.

alexarchambault and others added 5 commits July 15, 2025 17:46
These secrets might leak with Mill's prompt, that tries to print back
the command arguments. These might be truncated in particular, which
can make parts of these secrets slip through output secret detection.
 Conflicts:
	libs/javalib/src/mill/javalib/SonatypeCentralPublishModule.scala
@alexarchambault alexarchambault marked this pull request as ready for review July 15, 2025 20:07
@arturaz
Copy link
Collaborator

arturaz commented Jul 21, 2025

Perhaps we should make Mill support taking in mill.util.Secret[String] as the command argument, and that would prevent Mill from printing it out in any place?

@alexarchambault
Copy link
Collaborator Author

@arturaz Maybe at some point, yes. We need better secret management overall, some secrets might leak in serialized task results under out/ too. I think we should merge this in the mean time.

 Conflicts:
	libs/javalib/src/mill/javalib/SonatypeCentralPublishModule.scala
Comment on lines +298 to +305
val isCI = Task.env.get("CI").nonEmpty
if (!force && isCI && (usernameParameterValue.nonEmpty || passwordParameterValue.nonEmpty))
sys.error(
"--username and --password options forbidden on CI. " +
"Their use might leak secrets. " +
s"Pass those values via environment variables instead ($USERNAME_ENV_VARIABLE_NAME and $PASSWORD_ENV_VARIABLE_NAME), or pass --force alongside them. " +
"You might want to check the output of this job for a leak of those secrets or parts of them."
)
Copy link
Member

@lefou lefou Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we consider this unsafe, we might as well print the same message as warning in all other cases.

Also, instead of sys.error use Task.fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants