environment table needs to be whitelisted/blacklisted

when I use pytest-html in docker (with gitlab-ci) it needs a lot of environment variables set, some holds token and passwords. I do not want to show in my jenkins report to a wider audience.

It seems they are collected in the very first test, so adding this to a test is working using the request autofixture:

        # remove eventual gitlab settings
        for k in list(request.config._metadata.keys()):
            if re.search(r'^(GITLAB_|CI_)', k):
                del request.config._metadata[k]

But I like to do it in conftest.py

You can remove them using the pytest_metadata hook.

import pytest
def pytest_metadata(metadata):
    metadata.pop("password", None)

Also, make sure you're using the latest version of pytest-metadata (looks like you are 1.8.0), since we've removed some of the env vars that will always contain sensitive data: pytest-dev/pytest-metadata#18

EDIT: Come to think of it, I wonder why those aren't masked for you? 🤔 According the above PR, they should always be.

I'm happy to review a PR (to pytest-metadata) with additional env vars that should be removed.

Maybe I was too quick to close this. I wonder why some of those are shown, since they were removed in v1.8.0

I see why now, that PR was merged after the release of v1.8.0.

The question still stands tho: Why aren't they masked for you? Maybe because they're passed to a docker container and GitLab can't pick that up and mask them. 🤔

Anyway, I'll get started on a release that will solve some of them for you. For the others, you'll have to use the hook.

@krzysztof-pawlik-gat

GitLab passes those variables to jobs as they are, the masked thing is applied when GitLab shows job output only (stdout/stderr).

As I was saying then: Maybe because they're passed to a docker container and GitLab can't pick that up and mask them.

Keep in mind that there are useful variables, like GITLAB_USER_EMAIL shows who triggered the pipeline (no need to resolve GITLAB_USER_ID to a user name), CI_PROJECT_URL allows me to quickly visit affected repository (no need to copy & paste CI_PROJECT_PATH).

Agreed. I won't remove more than already removed in the linked PR. Unless a very compelling argument is put forth. Especially since, imo, it's very easy customizing what is shown and not using the aforementioned hook.

There's a huge difference between anonymizing the report for public show & tell and internal use.

Yes. Not sure what your point is? 🤔

@fenchu

The initial report from @fenchu shows some variables masked that in my opinion should not be masked or removed. My intention with this line was that usefulness of the report should not be sacrificed in the name of it being publicly shareable.

Yeah, I'm even contemplating reverting that PR and going about this is in a different way.

I see a couple of different approaches we can take, my favorite is to implement something similar to pytests filterwarnings where you can list the env vars you want masked in pytest.ini.

What are your thoughts @davehunt ?

Thanks for clarification :-)

Filtering manually is fine.

My initial question was more about doing this from the conftest.py file. if I use request or metadata fixtures they seem to run at the beginning before all my tests. Is there a hook so it can run after the testloop stage before reportgathering?

@fenchu

@fenchu
So there's both the metadata fixture and the pytest_metadata hook.

I would say that the fixture is used more to read metadata and the hook is used to modify.

So the hook should give you what you want - filtering out the sensitive data. Are you saying that is not working? 🤔

@davehunt

I see a couple of different approaches we can take, my favorite is to implement something similar to pytests filterwarnings where you can list the env vars you want masked in pytest.ini.

What are your thoughts @davehunt ?

This sounds like a reasonable approach to me. Perhaps we need two filters, one for variables to mark and another for variables to exclude completely. We could default to masking anything matching 'token' or 'password' in an effort to protect potentially vulnerable data, but we should be careful not to suggest that we're protecting these values. In fact, we should probably call out in the documentation that there is a risk of exposing secrets when using the plugin.

@BeyondEvil

Hey @BeyondEvil this sounds like an interesting thing we should implement, I'm going to get started on it.

The approach I'll take is:

I see a couple of different approaches we can take, my favorite is to implement something similar to pytests filterwarnings where you can list the env vars you want masked in pytest.ini.

Am I missing any information to start working on this?

@BeyondEvil

Hey @BeyondEvil this sounds like an interesting thing we should implement, I'm going to get started on it.

Wow, that'd be great!

The approach I'll take is:

I see a couple of different approaches we can take, my favorite is to implement something similar to pytests filterwarnings where you can list the env vars you want masked in pytest.ini.

Excellent!

Am I missing any information to start working on this?

I don't think so.

I'm going back and forth in my head thinking about where this belongs.

But I think I've landed in this:

pytest-metadata shouldn't mask anything, it's impossible to foresee the use-cases consumers of the library may have. They may actually want to get secret values. Besides, there's both a hook and fixture to manipulate the data.

pytest-html should provide one mechanism to redact values and one to remove them completely like Dave suggested.

If this makes sense to you @gnikonorov I will make a release of pytest-metadata where I deprecate the default masking.

pytest-html should make no assumptions of values being masked, I think.

@BeyondEvil

That makes sense @BeyondEvil. I'll implement this in two PRs then. One for redacting and one for masking.

Turns out I don't have to do anything with pytest-metadata, it doesn't mask or anything currently. The "secret" environment variables were simple removed from the respective CI vendors list.

@BeyondEvil

Starting work on the first part now, @BeyondEvil. I'll allow for users to specify a list of environment variables to remove from the table completely ( not mask ).

@gnikonorov

You're the best @gnikonorov 💪

@BeyondEvil

Question though, @BeyondEvil. Why not just have users delete items from the environment table ( e.g.:del session.config._metadata["foo"] )? Is the idea just to have a more convenient interface for users or is it not always possible to delete keys?

@gnikonorov

I actually don't know @gnikonorov

It's over a year old and not a request that is coming in very often.

Perhaps your time is better spent on other things? 🤔 I mean, there's already two ways of removing items from metadata, maybe that's enough? Up to you really.

@BeyondEvil

How about we implement redacting and close this issue once it's in master?

I was thinking we could let the user pass a list of strings/regexes and any metadata key that matches has its value redacted.

We could make it an INI file setting.

Thoughts @BeyondEvil ?

Navigation Menu

Repository navigation

environment table needs to be whitelisted/blacklisted #233

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

environment table needs to be whitelisted/blacklisted #233

Description

Activity

BeyondEvil commented on Aug 29, 2019

BeyondEvil commented on Aug 29, 2019

BeyondEvil commented on Aug 29, 2019

BeyondEvil commented on Aug 29, 2019

BeyondEvil commented on Aug 29, 2019

fenchu commented on Aug 29, 2019

BeyondEvil commented on Aug 29, 2019

davehunt commented on Sep 11, 2019

gnikonorov commented on Dec 15, 2020

BeyondEvil commented on Dec 15, 2020

gnikonorov commented on Dec 15, 2020

BeyondEvil commented on Dec 15, 2020

gnikonorov commented on Dec 18, 2020

BeyondEvil commented on Dec 18, 2020

gnikonorov commented on Dec 18, 2020

BeyondEvil commented on Dec 19, 2020

gnikonorov commented on Dec 19, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions