cockpit: support setting owner/group in fsreplace1

Cockpit-files wants to support uploading or creating a file owned by the
current directory which might be different from the logged in user.

For example as superuser uploading a database into `/var/lib/postgresql`
which would be owned by `postgres` and the database file should receive
the same permissions.

doc/protocol.md

@@ -1072,6 +1072,11 @@ The following options can be specified in the "open" control message:
    you don't set this field, the actual tag will not be checked.  To
    express that you expect the file to not exist, use "-" as the tag.
 
+* "attrs": a JSON object containing file owner and group information
+           to set. Both `owner` and `group` are required.
+  - `owner`: a string, or an integer, the uid of the file owner (`st_uid`)
+  - `group`: a string, or an integer, the gid of the file group (`st_gid`)
+
 You should write the new content to the channel as one or more
 messages.  To indicate the end of the content, send a "done" message.
 

pkg/lib/cockpit.d.ts

@@ -257,9 +257,14 @@ declare module 'cockpit' {
         remove(): void;
     }
 
+    interface ReplaceAttrs {
+      owner: string | number;
+      group: string | number;
+    }
+
     interface FileHandle<T> {
         read(): Promise<T>;
-        replace(new_content: T | null, expected_tag?: FileTag): Promise<FileTag>;
+        replace(new_content: T | null, expected_tag?: FileTag, attrs?: ReplaceAttrs): Promise<FileTag>;
         watch(callback: FileWatchCallback<T>, options?: { read?: boolean }): FileWatchHandle;
         modify(callback: (data: T | null) => T | null, initial_content?: string, initial_tag?: FileTag): Promise<[T, FileTag]>;
         close(): void;

pkg/lib/cockpit.js

@@ -2241,7 +2241,7 @@ function factory() {
 
         let replace_channel = null;
 
-        function replace(new_content, expected_tag) {
+        function replace(new_content, expected_tag, attrs) {
             const dfd = cockpit.defer();
 
             let file_content;
@@ -2259,8 +2259,12 @@ function factory() {
                 ...base_channel_options,
                 payload: "fsreplace1",
                 path,
-                tag: expected_tag
+                tag: expected_tag,
             };
+
+            if (attrs)
+                opts.attrs = attrs;
+
             replace_channel = cockpit.channel(opts);
 
             replace_channel.addEventListener("close", function (event, message) {

pkg/playground/test.html

@@ -45,6 +45,19 @@
 			<p>cockpit.user() information</p>
 			<div id="user-info" />
                 </div>
+                <br/>
+                <div id="fsreplace1-div">
+                    <h2>fsreplace1 test</h2>
+                    <p>new filename</p>
+                    <input id="fsreplace1-filename" />
+                    <p>text content</p>
+                    <input id="fsreplace1-content" />
+                    <p>owner mode</p>
+                    <input id="fsreplace1-owner" placeholder="For example, admin" />
+                    <input id="fsreplace1-group" placeholder="For example, users" />
+                    <button id="fsreplace1-create" class="pf-v5-c-button pf-m-secondary">Create file</button>
+                    <div id="fsreplace1-error"></div>
+                </div>
             </section>
         </main>
     </div>

pkg/playground/test.js

@@ -130,6 +130,30 @@ document.addEventListener("DOMContentLoaded", () => {
         document.getElementById("user-info").textContent = JSON.stringify(info);
     });
 
+    const fsreplace_btn = document.getElementById("fsreplace1-create");
+    const fsreplace_error = document.getElementById("fsreplace1-error");
+    fsreplace_btn.addEventListener("click", e => {
+        fsreplace_btn.disabled = true;
+        fsreplace_error.textContent = '';
+        const filename = document.getElementById("fsreplace1-filename").value;
+        const content = document.getElementById("fsreplace1-content").value;
+        const attrs = { };
+        for (const field of ["owner", "group"]) {
+            const val = document.getElementById(`fsreplace1-${field}`).value;
+            if (!val)
+                continue;
+
+            attrs[field] = val;
+        }
+        cockpit.file(filename, { superuser: "try" }).replace(content, undefined, attrs)
+                .catch(exc => {
+                    fsreplace_error.textContent = exc.toString();
+                })
+                .finally(() => {
+                    fsreplace_btn.disabled = false;
+                });
+    });
+
     cockpit.addEventListener("visibilitychange", show_hidden);
     show_hidden();
 });

src/cockpit/channels/filesystem.py

@@ -43,7 +43,9 @@
     JsonObject,
     get_bool,
     get_int,
+    get_object,
     get_str,
+    get_str_or_int,
     get_strv,
     json_merge_and_filter_patch,
 )
@@ -158,6 +160,50 @@ def do_yield_data(self, options: JsonObject) -> Generator[bytes, None, JsonObjec
             raise ChannelError('internal-error', message=str(exc)) from exc
 
 
+class FSReplaceAttrs:
+    uid: 'int | None' = None
+    gid: 'int | None' = None
+    supported_attrs = ['owner', 'group']
+
+    def __init__(self, value: JsonObject) -> None:
+        # Any unknown keys throw an error
+        unsupported_attrs = list(value.keys() - self.supported_attrs)
+        if unsupported_attrs:
+            raise ChannelError('protocol-error',
+                               message=f'"attrs" contains unsupported key(s) {unsupported_attrs}') from None
+
+        try:
+            owner = get_str_or_int(value, 'owner', None)
+        except JsonError:
+            raise ChannelError('protocol-error', message='"owner" must be an integer or string') from None
+
+        try:
+            group = get_str_or_int(value, 'group', None)
+        except JsonError:
+            raise ChannelError('protocol-error', message='"group" must be an integer or string') from None
+
+        if owner is not None and group is None:
+            raise ChannelError('protocol-error', message='"group" attribute is empty while "owner" is provided')
+        if group is not None and owner is None:
+            raise ChannelError('protocol-error', message='"owner" attribute is empty while "group" is provided')
+
+        if isinstance(owner, str):
+            try:
+                self.uid = pwd.getpwnam(owner).pw_uid
+            except KeyError:
+                raise ChannelError('internal-error', message=f'uid not found for {owner}') from None
+        else:
+            self.uid = owner
+
+        if isinstance(group, str):
+            try:
+                self.gid = grp.getgrnam(group).gr_gid
+            except KeyError:
+                raise ChannelError('internal-error', message=f'gid not found for {group}') from None
+        else:
+            self.gid = group
+
+
 class FsReplaceChannel(AsyncChannel):
     payload = 'fsreplace1'
 
@@ -168,10 +214,18 @@ def delete(self, path: str, tag: 'str | None') -> str:
             os.unlink(path)
         return '-'
 
-    async def set_contents(self, path: str, tag: 'str | None', data: 'bytes | None', size: 'int | None') -> str:
+    async def set_contents(self, path: str, tag: 'str | None', data: 'bytes | None', size: 'int | None',
+                           attrs: 'FSReplaceAttrs | None') -> str:
         dirname, basename = os.path.split(path)
         tmpname: str | None
         fd, tmpname = tempfile.mkstemp(dir=dirname, prefix=f'.{basename}-')
+
+        def chown_if_required(fd: 'int'):
+            if attrs is None or attrs.uid is None or attrs.gid is None:
+                return
+
+            os.fchown(fd, attrs.uid, attrs.gid)
+
         try:
             if size is not None:
                 logger.debug('fallocate(%s.tmp, %d)', path, size)
@@ -195,12 +249,14 @@ async def set_contents(self, path: str, tag: 'str | None', data: 'bytes | None',
                 # no preconditions about what currently exists or not
                 # calculate the file mode from the umask
                 os.fchmod(fd, 0o666 & ~my_umask())
+                chown_if_required(fd)
                 os.rename(tmpname, path)
                 tmpname = None
 
             elif tag == '-':
                 # the file must not exist.  file mode from umask.
                 os.fchmod(fd, 0o666 & ~my_umask())
+                chown_if_required(fd)
                 os.link(tmpname, path)  # will fail if file exists
 
             else:
@@ -225,22 +281,23 @@ async def run(self, options: JsonObject) -> JsonObject:
         path = get_str(options, 'path')
         size = get_int(options, 'size', None)
         tag = get_str(options, 'tag', None)
+        attrs = get_object(options, 'attrs', FSReplaceAttrs, None)
 
         try:
             # In the `size` case, .set_contents() sends the ready only after
             # it knows that the allocate was successful.  In the case without
             # `size`, we need to send the ready() up front in order to
             # receive the first frame and decide if we're creating or deleting.
             if size is not None:
-                tag = await self.set_contents(path, tag, b'', size)
+                tag = await self.set_contents(path, tag, b'', size, attrs)
             else:
                 self.ready()
                 data = await self.read()
                 # if we get EOF right away, that's a request to delete
                 if data is None:
                     tag = self.delete(path, tag)
                 else:
-                    tag = await self.set_contents(path, tag, data, None)
+                    tag = await self.set_contents(path, tag, data, None, attrs)
 
             self.done()
             return {'tag': tag}

src/cockpit/jsonutil.py

@@ -87,6 +87,15 @@ def get_str_or_none(obj: JsonObject, key: str, default: Optional[str]) -> Option
     return _get(obj, lambda v: None if v is None else typechecked(v, str), key, default)
 
 
+def get_str_or_int(obj: JsonObject, key: str, default: Optional[Union[str, int]]) -> Optional[Union[str, int]]:
+    def as_str_or_int(value: JsonValue) -> Union[str, int]:
+        if not isinstance(value, (str, int)):
+            raise JsonError(value, 'must be a string or integer')
+        return value
+
+    return _get(obj, as_str_or_int, key, default)
+
+
 def get_dict(obj: JsonObject, key: str, default: Union[DT, _Empty] = _empty) -> Union[DT, JsonObject]:
     return _get(obj, lambda v: typechecked(v, dict), key, default)
 

test/pytest/test_bridge.py

@@ -14,7 +14,7 @@
 import unittest.mock
 from collections import deque
 from pathlib import Path
-from typing import Dict, Iterable, Iterator, Sequence
+from typing import Dict, Generator, Iterable, Iterator, Optional, Sequence, Union
 
 import pytest
 
@@ -482,6 +482,28 @@ async def test_fslist1_notexist(transport: MockTransport) -> None:
         reply_keys={'message': "[Errno 2] No such file or directory: '/nonexisting'"})
 
 
+class GrpPwMock:
+    def __init__(self, uid: int, gid: Optional[int] = None) -> None:
+        self.pw_uid = uid
+        self.gr_gid = gid
+
+    def __str__(self) -> str:
+        return f'uid={self.pw_uid},gid={self.gr_gid}'
+
+
+@pytest.fixture
+def fchown_mock():
+    with unittest.mock.patch('os.fchown', return_value=True) as fchown_mock:
+        yield fchown_mock
+
+
+@pytest.fixture
+def owner_group_mock(owner_group_mock_arg: GrpPwMock) -> Generator[GrpPwMock, GrpPwMock, None]:
+    with unittest.mock.patch('pwd.getpwnam', return_value=owner_group_mock_arg):
+        with unittest.mock.patch('grp.getgrnam', return_value=owner_group_mock_arg):
+            yield owner_group_mock_arg
+
+
 @pytest.mark.asyncio
 async def test_fsreplace1(transport: MockTransport, tmp_path: Path) -> None:
     # create non-existing file
@@ -655,6 +677,83 @@ async def test_fsreplace1_error(transport: MockTransport, tmp_path: Path) -> Non
                                    'message': """attribute 'send-acks': invalid value "not-valid" not in ['bytes']"""
     })
 
+    await transport.check_open('fsreplace1', path=str(tmp_path / 'test'),
+                               attrs={'owner': 'cockpit', 'group': 'cockpit', 'selinux': True},
+                               problem='protocol-error',
+                               reply_keys={
+                                   'message': '"attrs" contains unsupported key(s) [\'selinux\']'
+    })
+
+    await transport.check_open('fsreplace1', path=str(tmp_path / 'test'), attrs={'owner': 'cockpit'},
+                               problem='protocol-error',
+                               reply_keys={
+                                   'message': '"group" attribute is empty while "owner" is provided'
+    })
+
+    await transport.check_open('fsreplace1', path=str(tmp_path / 'test'), attrs={'group': 'cockpit'},
+                               problem='protocol-error',
+                               reply_keys={
+                                   'message': '"owner" attribute is empty while "group" is provided'
+    })
+
+    await transport.check_open('fsreplace1', path=str(tmp_path / 'test'), attrs={'owner': 'cockpit', 'group': []},
+                               problem='protocol-error',
+                               reply_keys={
+                                   'message': '"group" must be an integer or string'
+    })
+
+    await transport.check_open('fsreplace1', path=str(tmp_path / 'test'), attrs={'owner': [], 'group': 'test'},
+                               problem='protocol-error',
+                               reply_keys={
+                                   'message': '"owner" must be an integer or string'
+    })
+
+    mock_val = GrpPwMock(uid=0, gid=1000)
+    with unittest.mock.patch('pwd.getpwnam', side_effect=KeyError()):
+        await transport.check_open('fsreplace1', path=str(tmp_path / 'test'),
+                                   attrs={'owner': 'bazinga', 'group': 'foo'},
+                                   problem='internal-error',
+                                   reply_keys={
+                                       'message': 'uid not found for bazinga'
+        })
+
+    with unittest.mock.patch('pwd.getpwnam', return_value=mock_val):
+        with unittest.mock.patch('grp.getgrnam', side_effect=KeyError()):
+            await transport.check_open('fsreplace1', path=str(tmp_path / 'test'),
+                                       attrs={'owner': 'bazinga', 'group': 'test'},
+                                       problem='internal-error',
+                                       reply_keys={
+                                           'message': 'gid not found for test'
+            })
+
+
+ATTRS_TEST_DATA = [
+    (GrpPwMock(1111, 1110), {'owner': 1111, 'group': 1110}),
+    (GrpPwMock(1111, 1110), {'owner': 'monkey', 'group': 'group'}),
+]
+
+
+@pytest.mark.parametrize(('owner_group_mock_arg', 'attrs'), ATTRS_TEST_DATA)
+@pytest.mark.asyncio
+async def test_fsreplace1_attrs(transport: MockTransport, fchown_mock: unittest.mock.MagicMock,
+                                tmp_path: Path, owner_group_mock, attrs) -> None:
+    async def create_file_with_attrs(filename: str, attrs: Dict[str, Union[int, str]]) -> None:
+        test_file = str(tmp_path / filename)
+        ch = await transport.check_open('fsreplace1', path=str(test_file), attrs=attrs)
+        transport.send_data(ch, b'content')
+        transport.send_done(ch)
+        await transport.assert_msg('', command='done', channel=ch)
+        await transport.check_close(ch)
+
+    await create_file_with_attrs('test', attrs)
+    fchown_mock.assert_called()
+    _, call_uid, call_gid = fchown_mock.call_args[0]
+    assert call_uid == owner_group_mock.pw_uid
+    if owner_group_mock.gr_gid is None:
+        assert call_gid == owner_group_mock.pw_uid
+    else:
+        assert call_gid == owner_group_mock.gr_gid
+
 
 @pytest.mark.asyncio
 async def test_fsreplace1_size_hint(transport: MockTransport, tmp_path: Path) -> None: