[Feature]: prevent log filled input values of type=password in HTML report

[d-koppenhagen]

🚀 Feature Request

As discussed in the comments in #35450, I will open an issue for preventing logs of probably sensitive input data.

#35457 introduced logs for input data in playwright reports.
This feature may causes unintentionally logged/reported secrets.
It can be a major problem, especially for automated login tests, and it should be ensured that input[type=password] is not logged or not logged in plain text (e.g., ******).

Of course, test should NOT use credentials use for production environments etc.
But even dedicated credentials for a test user with very limited permissions can be sensitive.

Independently, once noticed the credentials were leaked (due to sent report or available pipeline artifacts / logs), credentials should be rotated.
And of course there are still other inputs which are my sensitive but not of type "password".

But I think it would prevent pitfalls when at least input[type=password] will be hidden in the logs since here it is very clear that we usually don't want to log thins kind of information.

Example

The feature should be activated by default.
Optionally a config option to disable this feature could be helpful

Motivation

Prevent pitfalls and unintentionally leaked passwords

[d-koppenhagen]

A workaround to prevent logs for a specific (e. g. password) field is:

await page.getByLabel('Password').evaluate((input, password) => {
  input.value = password;
}, process.env.MY_PASSWORD!);

[mxschmitt]

Folding into #19992 - we recommend uploading it on a secure storage / adding auth / encrypting it.

[gjmargallo]

I don't understand how this can be closed without further analysis.

It shows a completely lack of understanding of security company requirements in the testing domain, and forces us not to use trace viewer, when this is the best capability of the solution.

It is only necessary to add a new option in the locator area to prevent logging...

[Skn0tt]

Hi folks! I asked Danny to open this as a separate issue, so we have something to track the request. I wasn't aware of the existing issue about it, but it makes sense to me. #19992 (comment) explains the general rationale towards security in the trace viewer, and I think it applies to this issue very well.

Hiding type=password is a heuristic that works for some cases, but it'd create a false sense of security for the others. So if you care about not leaking credentials, apply the same care to not leaking those HTML reports and trace viewer files.

[Skn0tt]

Could you share your workflow that incorporates sending around HTML reports and trace viewer files? Are you sending them to untrusted parties?

[Morl99]

I can share the setup we (includes Danny and probably hundres of other Developers at Deutsche Bahn) have:
We run playwright tests in GitLab CI pipeplines and the resulting report is saved as a GitLab CI Artefact. Storing or exposing secrets within GitLab CI Artefacts is prohibited.

I second the idea, that password field values should be masked. Even if this does not give 100% guarantee, I do believe, that everyone that sees **** in a report for a password field would understand this kind of behaviour.

Maybe it helps to separate this issue from #19992. This issue is about secrets in the report, originating from values in password fields filled with fill introduced by the recent #35457. #19992 is about trace files and the discussion is a lot older.

Mandating that playwright reports have to be handled with care (encrypted, classified) would really hurt the developer experience. I want to share those reports and make them accessible.

[Skn0tt]

Thanks for sharing your workflow. I don't think these issues can be separated. Showing fill(password) is an obvious credential leak, and we can fix it - but there's other leaks that are impossible for us to fix. See #19992 (comment).

So if we mask password fields in the report, it would still be just as sensitive. The leak just wouldn't be obvious anymore.
The only solution to this is to handle your reports with care. Either by switching to a trusted artifact store (and placing links to that in your Gitlab CI results), or by encrypting the artifacts before uploading.

[adiljaura]

Completely agree with @Morl99, this does feel separate to #19992, in that this is an issue after the changes made in #35457 and more related to the html report rather than trace.

Prior to v1.52.0, we've not had an issue with credential leaks in our workflow. We're keen to keep html reports easily accessible for stakeholders and the wider team. Traces, for us at least, are not widely shared.

Appreciate the points raised around it being impossible to fix all leaks but this is one that is newly introduced, and if it's easily fixed, then I think it makes sense to do this.

[d-koppenhagen]

I understand the argument that it is not Playwright's responsibility to ensure that sensitive data can be read out in fields when input. However, I cannot understand the argument that this is why there is no attempt to reveal them at any point.

I think it would make sense to prevent explicit reporting at least in clear cases such as input[type=password]. The reports are often used for error analysis and are essential, especially in CI/CD pipelines. Encryption of the entire report or the upload to an external service cause a massive deterioration of the developer experience.

In my view, the implementation and also the argumentation are also in contrast to otherwise really excellent documentation. There is explicitly the example with input of user/password under on a separate page for authentication.

If the feature is not implemented, I would like a thick disclaimer at least in the documentary, which indicates that sensitive data may be part of the traces or reports and can thus be exposed unwanted. I would also like a note on best practice that describes how to deal with it.

[Skn0tt]

My argument is not that it's not Playwright's responsibility - we'd love to make the report secure! I think I need to understand the security approach you're coming from better.

When you say "Storing or exposing secrets within GitLab CI Artefacts is prohibited.", is that a corporate policy because your organisation doesn't trust the GitLab Artifacts in particular, or is it based on something GitLab advises in their documentation?

Playwright could definitely hide the input value for type=password fields, but there's other leaks we won't be able to fix in the HTML report. Some examples:

• error messages containing API keys
• source snippets containing hard-coded tokens or passwords

In the trace viewer, there's a myriad more:

• Authentication headers, both standard and non-standard
• cookies
• console.log lines containing tokens
• redirect URLs with query parameters that contain tokens
• DOM snapshots with input fields containing a password

If we go ahead and implement the type=password change in HTML report you're mentioning, those leaks above would remain. They might not be visible when opening the report, but when scanning the html files, there's a good chance you could find them in the raw data. Would you still be comfortable uploading the HTML reports and traces to Github Artifacts then? Those leaks existed before, and there's a good chance they also affected your app.

[Morl99]

We have a company policy that specifies where and how secrets are allowed to be handled. GitLab Artifacts is not on that list (unless of course they are encrypted, which would defeat the purpose of such a report).

The reason we opened this bug is, that we have secret scanners that found leaks across teams all related to the change in #35457. So your hypotheses that some of the other problems affect us seems to be false, because otherwise those scans would most likely show these findings as well.

I appreciate the open discussion about this topic, thank you for that.

I personally believe that fixing this issue would improve the behavior of Playwright and would move it into a direction of what people would expect, even if this will not solve all potential issues with leaked secrets.

And if someone has hardcoded tokens or passwords in code, they will probably not mind if they also show up in a test report... (I hope we agree, that tokens or passwords should not be out into source code)

[Skn0tt]

I appreciate the open discussion about this topic, thank you for that.

Sorry for closing it a little to early before. I'm happy to discuss it, it's not often that I get the chance to work with my favourite railway operator!

So your hypotheses that some of the other problems affect us seems to be false, because otherwise those scans would most likely show these findings as well.

Traces also contain the fill contents, even before #35457. There's a good chance you were affected and your secret scanner didn't find them because they're stored in zip compression.

fixing this issue [...] would move [Playwright] into a direction of what people would expect

Let me try to make the opposite argument. Since HTML Report and Traces will always contain sensitive data in its internals, it's a good thing we make this very apparent by showing those sensitive fields to the user. Instead of pretending that Playwright can meet the users expectations, this moves the users expectation into the direction of what we can achieve technically.

On your side, my recommendation is to work towards trusting your artifact repository, and in the meantime encrypting the report. I don't think it defeats the purpose of the report; write yourself a small helper bash script that decrypts the report before opening it.

On Playwright's side, I'll amend the docs to mention that reports and traces contain sensitive data.

[philipgriffin]

I have been investigating a solution for this but it makes absolutely no sense to me for Playwright to offer features to mask secrets in trace reports.

Applications could handle secrets in many different ways outside of inputs alone (ie cookies/headers/req/resp and so on).

If you want secrets maintained upload the report to a secure location or if you want secrets masked do so programmatically. A basic example to mask secrets with a uuid (presuming strongly-unique secrets) here

[Skn0tt]

We discussed this with the team and arrived at adding a docs page about "Secrets" that discusses how any Playwright output contains sensitive data like secrets or source code.

[ethan-bm]

I agree with @Morl99 and @adiljaura.

The problem my company is facing is easily shareable HTML reports with stakeholders.

Our CI workflow only uploads screenshots and videos, but not traces, to our HTML reports. Exposing input values, while not offering a mask option, is something our compliance team has flagged as a major issue.

Playwright could definitely hide the input value for type=password fields, but there's other leaks we won't be able to fix in the HTML report. Some examples:
error messages containing API keys
source snippets containing hard-coded tokens or passwords

Imho, for HTML reports only, those are not concerns with a password input field being exposed. Those seem more related to best practices/process. How do these concerns prevent offering a mask input option?

error messages containing API keys

• We upload our results to a reporting tool, which is handled in a GitHub action job after the tests have run. Our E2E tests do not call secured API endpoints with tokens.

source snippets containing hard-coded tokens or passwords

• We use process.env to pull from GitHub secrets in all our E2E tests in CI.

Furthermore, how is it that release v1.52 exposed inputs like this without any mention in the release notes? It has caused massive headaches in my company as we slowly discovered exposed passwords in our json, html, and test reports in our reporting software.

Please consider offering a mask.input option. Even if it has to have a disclaimer that it does not secure the reports, please at least provide the option for those that know their use cases.

[philipgriffin]

@ethan-bm I don't see a point in adding a mask if it does not offer any security..? Rather it does the opposite and creates a false sense of security encouraging others to share your reports leading to exposed secrets. What is stopping you just cleaning the report before pushing it?

[ethan-bm]

@ethan-bm I don't see a point in adding a mask if it does not offer any security..? Rather it does the opposite and creates a false sense of security encouraging others to share your reports leading to exposed secrets. What is stopping you just cleaning the report before pushing it?

@philipgriffin
Where in my HTML report is a secret being exposed, outside of the list of steps from the "Test Steps" section?

• In our playwright.config.ts, we turn off traces on CI in the "use" section with trace: process.env.CI ? 'off':'retain-on-first-failure',
• I opened up the HTML report folder, opened the index.html file in VSC as raw data, and searched for any exposed passwords from input.fill(password). It was not there.
• The only data in the "data" folder of the HTML report were three screenshots of windows taken at the end of the test.
• No trace file was uploaded to HTML report or in the artifact we create.
• No json of raw step output was uploaded in my artifact or my html report.
• The test case itself has 0 hardcoded passwords or API tokens.
  • We use process.env to pull our passwords from our private company repository's GitHub secrets.
• The actual test does not call out to any endpoints with tokens.

Furthermore, this is a functionality change specifically introduced in v1.52, which changed the behavior/view of test steps in HTML reports. It used to just show locator.getByPlaceholder('Password').fill. Now it is locator.getByPlaceholder('Password').fill(exposedPasswordValue). I don't think it's unreasonable to ask the company who makes the framework that features they created be extended into new features to maintain parity. Especially given the high amount of feedback this has received from the community. Sure, I can work on creating a work around, but I prefer to appeal to the Playwright devs first.

Edit: To prove my point, look at the massive amount of comments here. Note: Trace != html report. I understand many more security concerns are exposed in the trace. I just wanted to point out this is a highly discussed topic.

[Skn0tt]

As per my comment above, we merged an addition to the docs that calls out that reports and traces contain sensitive data. If you know your usecases and still need to mask some of your inputs, then you can use the workaround shared in #35848 (comment).

I'll go ahead and close this issue. I understand how traces appear to be different from the HTML report at first sight, but from a technical perspective the data they contain is very similar, and so the security rationale to apply is the same. As we shared above, we believe that masking the input value creates a false sense of security because it covers the fact that these reports contain sensitive data.