Skip to content

player/command: add env property#17577

Open
N-R-K wants to merge 2 commits intompv-player:masterfrom
N-R-K:env-property
Open

player/command: add env property#17577
N-R-K wants to merge 2 commits intompv-player:masterfrom
N-R-K:env-property

Conversation

@N-R-K
Copy link
Copy Markdown
Contributor

@N-R-K N-R-K commented Mar 14, 2026

mainly so that env vars can be accessed from within input.conf.

@kasper93
Copy link
Copy Markdown
Member

kasper93 commented Mar 15, 2026

I don't think it's a good idea to allow environmental variables snooping. We already allow clipboard snooping and it isn't quite liked feature, see #17367.

@verygoodlee
Copy link
Copy Markdown
Contributor

I don't think it's a good idea to allow environmental variables snooping. We already allow clipboard snooping and it isn't quite liked feature, see #17367.

Yes, and this feature can be implemented by a simple lua script, it's not worth adding a property for it.

add this script, and then you can access environment variable through ${user-data/env/...} in input.conf

local utils = require('mp.utils')

local env = {}
for _, v in ipairs(utils.get_env_list()) do
    local i = v:find('=', 1, true)
    env[v:sub(1, i - 1)] = v:sub(i + 1)
end
mp.set_property_native('user-data/env', env)

@N-R-K
Copy link
Copy Markdown
Contributor Author

N-R-K commented Mar 15, 2026

We already allow clipboard snooping and it isn't quite liked feature

This isn't even remotely comparable.

  1. Clipboard is shared across processes, env is process local.
  2. The clipboard feature isn't liked because it's perma snooping, not due to the feature existing. This property doesn't perma "snoop", not that would make any difference anyways due to point 1.
  3. Clipboard snooping breaks password managers doing "one time paste", there's no such thing to break here due to point 1.
  4. It's already possible to "snoop" env vars, (see: utils.get_env_list() and verygoodlee's comment) this just gives an easy way to access them in {input,mpv}.conf in order to reference variables like $XDG_RUNTIME_DIR and others.

@guidocella
Copy link
Copy Markdown
Contributor

You can easily read environment variable in scripts of course, but config files are more convenient for users. (The above script doesn't work for mpv.conf).

Notably there are no ~~ paths for ~/.local/share and $XDG_RUNTIME_DIR.

This has been requested in #mpv a few times and the implementation is trivial so I think it's a fine addition.

Copy link
Copy Markdown
Member

@kasper93 kasper93 left a comment

Choose a reason for hiding this comment

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

I don't think unpredictable config behavior based on current runtime env is a good thing. But there seems to be strong need for this features, so it's fine.

@N-R-K N-R-K requested a review from na-na-hi March 25, 2026 15:11
@na-na-hi
Copy link
Copy Markdown
Contributor

  1. env is process local.

4. It's already possible to "snoop" env vars, (see: utils.get_env_list() and verygoodlee's comment) this just gives an easy way to access them in {input,mpv}.conf in order to reference variables like $XDG_RUNTIME_DIR and others.

JSON IPC clients previously cannot get the environment of mpv, now they can even when they belong to processes different from mpv and normally cannot access mpv's process local environment.

@N-R-K
Copy link
Copy Markdown
Contributor Author

N-R-K commented Mar 26, 2026

JSON IPC clients previously cannot get the environment of mpv, now they can even when they belong to processes different from mpv and normally cannot access mpv's process local environment.

{
    "command": {
        "name": "subprocess"
        "playback_only": false,
        "args": [
            "sh",
            "-c",
            "env > /tmp/mpv.env # or send wherever"
        ],
    }
}

Not to mention, you can read other processes' env from /proc as well. Env vars are simply not a "secure" source to begin with. So there's no security issue in "leaking" them.

N-R-K added 2 commits March 26, 2026 16:05
these properties cannot be used directly and require a
sub-property. document this explicitly.
mainly so that env vars can be accessed from within config files.
Copy link
Copy Markdown
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

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

LGTM

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.

6 participants