Skip to content

Conversation

@fmeum
Copy link
Collaborator

@fmeum fmeum commented Sep 30, 2025

This information is generally useful for all builds, not just those that run the workspace status action.

@fmeum
Copy link
Collaborator Author

fmeum commented Sep 30, 2025

@bazel-io fork 8.5.0

@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Sep 30, 2025
@fmeum fmeum requested a review from lberki September 30, 2025 15:04
@fmeum
Copy link
Collaborator Author

fmeum commented Sep 30, 2025

@bazel-io fork 7.7.0

@lberki
Copy link
Contributor

lberki commented Oct 14, 2025

I'd rather this not get merged. Reason being, as it is, there is no guarantee that the username and especially the hostname is consistent with what the workspace status action says.

In addition, there is the concern that this may leak these strings against the will of the user; I think it's not a very strong argument because Bazel makes zero explicit effort to hide these from the BES, though.

Maybe add an explicit field to the BES somewhere with this data? That would have the advantage of having the data available if --workspace_status_command=/bin/true or the like is used.

dws added a commit to dws/bazel that referenced this pull request Oct 27, 2025
Per a review comment (bazelbuild#27120 (comment))
on bazelbuild#27120, we here propose to send the hostname and username via the
BuildStarted BES message rather than the workspace status BES message.
@dws
Copy link
Contributor

dws commented Oct 27, 2025

Maybe add an explicit field to the BES somewhere with this data? That would have the advantage of having the data available if --workspace_status_command=/bin/true or the like is used.

@lberki Since the BuildStarted event already contains things like the working directory and the workspace directory, would you be agreeable to adding the hostname and username as additional fields in this event? I put up #27421 which does this.

@lberki
Copy link
Contributor

lberki commented Oct 27, 2025

Yep, that's a more reasonable approach, thanks for pre-emptively implementing it.

@fmeum fmeum closed this Oct 27, 2025
@fmeum fmeum deleted the include-host-user branch October 27, 2025 08:29
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Oct 27, 2025
copybara-service bot pushed a commit that referenced this pull request Oct 28, 2025
Per a review comment (#27120 (comment)) on #27120, we here propose to send the hostname and username via the BuildStarted BES message rather than the workspace status BES message.

Closes #27421.

PiperOrigin-RevId: 825070359
Change-Id: I6e8f2e13e90aa6276f629c1da630a5da8e2468e9
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Oct 28, 2025
Per a review comment (bazelbuild#27120 (comment)) on bazelbuild#27120, we here propose to send the hostname and username via the BuildStarted BES message rather than the workspace status BES message.

Closes bazelbuild#27421.

PiperOrigin-RevId: 825070359
Change-Id: I6e8f2e13e90aa6276f629c1da630a5da8e2468e9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-CLI Console UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants