Page MenuHomePhabricator

mwscript-k8s does not include an environment variable with the username of the executing user
Closed, ResolvedPublic

Description

Background

I spent some time looking into T393444: Wiki creations not being reported to newprojects list. The addWiki.php script we use to create new project sends an email to newprojects@lists.wikimedia.org to tell everyone that a new project has been created (currently broken, but that's being looked into in the linked task). The script's logic contains something like this:

// [...]
$user = getenv( 'SUDO_USER' );
$body = "A new wiki was created by $user at $time for \"$siteName\".\n" .
	"Once the wiki is fully set up, it'll be visible at $url";
// [...]
return UserMailer::send( $to, $from, "New wiki: $wiki", $body );

This code is supposed to find the username of whoever started it (using the SUDO_USER environment variable), and include it in the announcement email.

Problem

As of writing, mwscript-k8s does not populate this variable:

[urbanecm@deploy1003 ~]$ mwscript-k8s --attach shell.php -- --wiki=testwiki
⏳ Starting shell.php on Kubernetes as job mw-script.eqiad.eqb5bju6 ...
🚀 Job is running.
ℹ️ Expecting a prompt but don't see it? Due to a race condition, the beginning of the output might be missing. Try pressing enter.
📜 Attached to stdin/stdout:

> getenv('SUDO_USER')
= false

> ^D

   INFO  Ctrl+D.

[urbanecm@deploy1003 ~]$

This is presumably because we do not actually make use of sudo (and even if we did, it would be from an in-container user rather than the executing user's name on deploy1003).

I tried to check whether there is a comparable env variable that would contain the username, but it doesn't seem there is:

> $envs = getenv();
> foreach($envs as $key => $value) {
. if(str_contains($value, 'urbanecm')) {
. var_dump([$key, $value]);
. break;
. }
. }
>
Proposed solution

Inject an environment variable that would include the name of the shell user who initiated the script.

Alternatively, considering addWiki.php seems to be the only user of SUDO_USER in MediaWiki, we might also elect to remove the name from the announcement instead.

Event Timeline

@RLazarus Do you have any thoughts on how to tackle this one, please?

This functionality was added 10 years ago in https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/WikimediaMaintenance/+/a17c2ef30e0e85ced460f304cf481cdb7d924486%5E%21

The commit message doesn't explain why the functionality of reporting the user creating the wiki was added, nor does it link to a task that would. The link changes from the commit message have nothing to do with figuring out who launched the script. However, digging in my emails I reach https://gerrit.wikimedia.org/r/c/operations/puppet/+/218766, which removes the Puppet populated shell script that was used until 2015 to do this. The farthest back I am able to go in time regarding this script is https://gerrit.wikimedia.org/r/plugins/gitiles/operations/puppet/+/e38a67b80625613ab4d47e297513bff865b0b869, from 2011, which is just putting into version control scripts that already existed without questioning why.

After 14+ years and a hugely altered WMF, it's prudent to just ask if the recipients of the mailing list see any advantage of knowning who run the script.

My guess is that it was a mechanism to create an audit trail. It made sense in that much much smaller organization that lacked other audit trail mechanisms. These days, those do exist, whether it's SAL, phab tasks, git/gerrit changes, they all serve the same purpose and more.

Just remove the functionality. It's probably faster and cheaper than trying to replicate it, just for the sake of carrying it forward.

BTW, on the technical side, mw-script does indeed keep the username in the labels of the job and the pod, e.g.

akosiaris@deploy1003:~$ kubectl describe pods mw-script.eqiad.zu6ggyo9-pftbw
Name:         mw-script.eqiad.zu6ggyo9-pftbw
Namespace:    mw-script
Priority:     0
Node:         wikikube-worker1088.eqiad.wmnet/10.64.0.58
Start Time:   Tue, 12 Aug 2025 12:55:13 +0000
Labels:       app=mediawiki
              controller-uid=c68c8126-3e6a-49f3-8a7d-0cb4132dac26
              deployment=mw-script
              job-name=mw-script.eqiad.zu6ggyo9
              release=zu6ggyo9
              routed_via=zu6ggyo9
              script=purgeList.php
              username=dbrant

Note the username label

So if we really need this information at some point, it is available. I just don't see a good reason (yet?) to implement exposing this as an env var for this specific use case.

I agree with @akosiaris (and thanks for the archaeology). It wouldn't be hard to implement this, but I think it's the wrong approach -- especially if addWiki.php is the only script using SUDO_USER, we should update the script rather than add an anachronism to pretend we're still using sudo.

If after reviewing that email functionality with fresh eyes (imagine it didn't already exist; would you build it today?) you decide it's still the best solution, let me know and I'll add a MWSCRIPT_USER variable. But I do think, from where I'm sitting, the SAL and other audit paths listed ought to be plenty.

I think we are mixing two questions here:

  1. Should we have the email to begin with (aka should we even have the newprojects@ list in general)?
  2. Assuming the email notification should exist, should it contain the shell username of whoever made the wiki?

The answer to the first question is a clear yes. The purpose of newprojects@ is not auditing, but notifying. There are certain teams and individuals that want to know we created a new project. This includes both cases of "I need to do something when a new wiki gets created" and cases of "I'm just excited when a new wiki is born". Ideally, we would live in a world when creating a wiki is fully automated, and there are no manual steps to do left to do after the wiki exists, but we are very, very far from that. Some things are handled by the post-creation task a bot creates in Phabricator, but it does not cover everything.

The second question is trickier. On one hand, I can understand we have the username in many other places already. On the other hand, if you start from the email notification itself, none of those places is easily accessible, mostly because the creation task is not linked. I can solve that by adding something like --phab to the script's params, which might be nicer than doing something special in k8s.

Does this make sense?

Sorry yes, I wrote that misleadingly, but I think @akosiaris and I are both addressing the question of whether the username needs to be in the email body. No objections to sending an email notification.

Mentioning the Phab task sounds like a good solution! If mwscript-k8s is invoked with --sal, it also guarantees that the username will appear in the task (via Stashbot comment).

Re-reading my first response, I realize I might have been a bit unclear. Indeed my focus was to respond to number 2, namely should it contain the shell username of whoever made the wiki? , not to question sending the email in the first place. I agree that this should still be sent. It's a notification as you say, not an auditing mechanism.

mostly because the creation task is not linked. I can solve that by adding something like --phab to the script's params, which might be nicer than doing something special in k8s.

I think you nailed it here. Even with the username being in the email notification, the linking part was still missing and an interested recipient of the notification has to dig a bit on their own to figure out where everything is tracked. Adding a --phab would create that missing link.

Clement_Goubert claimed this task.
Clement_Goubert triaged this task as Medium priority.
Clement_Goubert edited projects, added: ServiceOps new; removed: serviceops-deprecated.
Clement_Goubert subscribed.

Triage comments:

From the linked task T393444: Wiki creations not being reported to newprojects list it would seem like the issue of the notification has been resolved. I'm going to be bold and set this task to resolved as well, but feel free to reopen if I'm wrong.

If adding phab logging to mwscript-k8s is still something we want (and I think it's a good idea), it should probably be tracked in its own separate task that I'm going to create right now.