diff --git a/docs/release-notes/snapcraft-9-0.rst b/docs/release-notes/snapcraft-9-0.rst index 08ea8bf43a..dec076e5bb 100644 --- a/docs/release-notes/snapcraft-9-0.rst +++ b/docs/release-notes/snapcraft-9-0.rst @@ -324,6 +324,9 @@ Snapcraft 9.0.0 build packages with versioned dependencies couldn't be resolved. - `craft-parts#1492 `__ The :ref:`craft_parts_poetry_plugin` didn't work for core26 snaps. +- `#6301 `__ Using + ``export-login`` on a credentials file that already exists failed with an + internal error. Contributors diff --git a/pyproject.toml b/pyproject.toml index b6f741a29d..150c96fd2a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,6 +32,7 @@ dependencies = [ "setuptools>=69.0,<80.9.0", "snap-helpers", "tabulate", + "tomli>=2.4.0", "typing-extensions", "validators>=0.28.3", ] diff --git a/snapcraft/commands/account.py b/snapcraft/commands/account.py index 2a025bd33e..8fb71fd45d 100644 --- a/snapcraft/commands/account.py +++ b/snapcraft/commands/account.py @@ -221,6 +221,16 @@ def run(self, parsed_args: argparse.Namespace) -> None: # This is sensitive-- it should only be accessible by the owner private_open = functools.partial(os.open, mode=0o600) + # If the file already exists from a previous export, it was made + # owner-read-only by the chmod below. The opener's mode only applies + # when creating a new file, so an existing read-only file would fail + # to open for writing. Restore owner write access first. Limit this + # to regular files so an unusual login_file (e.g. a directory) isn't + # mutated unexpectedly. + login_path = pathlib.Path(parsed_args.login_file) + if login_path.is_file(): + login_path.chmod(stat.S_IRUSR | stat.S_IWUSR) + with open( parsed_args.login_file, "w", opener=private_open, encoding="utf-8" ) as login_fd: diff --git a/tests/unit/commands/test_account.py b/tests/unit/commands/test_account.py index 3f32f1c72a..54ebd1dcde 100644 --- a/tests/unit/commands/test_account.py +++ b/tests/unit/commands/test_account.py @@ -156,6 +156,33 @@ def test_export_login_file(project_path, emitter, fake_store_login, fake_app_con assert login_file.read_text() == "secret" +def test_export_login_file_overwrite( + project_path, emitter, fake_store_login, fake_app_config +): + """Re-exporting to an existing read-only credentials file should succeed.""" + cmd = commands.StoreExportLoginCommand(fake_app_config) + namespace = argparse.Namespace( + login_file="target_file", + snaps=None, + channels=None, + acls=None, + expires=None, + experimental_login=False, + ) + + cmd.run(namespace) + + login_file = project_path / "target_file" + # The first run leaves the file owner-read-only. + assert login_file.stat().st_mode & 0o777 == 0o400 + + # A second run must not fail with PermissionError. + cmd.run(namespace) + + assert login_file.read_text() == "secret" + assert login_file.stat().st_mode & 0o777 == 0o400 + + def test_export_login_with_params(emitter, fake_store_login, fake_app_config): cmd = commands.StoreExportLoginCommand(fake_app_config) diff --git a/uv.lock b/uv.lock index 813501c361..201584f14a 100644 --- a/uv.lock +++ b/uv.lock @@ -2419,6 +2419,7 @@ dependencies = [ { name = "setuptools" }, { name = "snap-helpers" }, { name = "tabulate" }, + { name = "tomli" }, { name = "typing-extensions" }, { name = "validators" }, ] @@ -2541,6 +2542,7 @@ requires-dist = [ { name = "setuptools", specifier = ">=69.0,<80.9.0" }, { name = "snap-helpers" }, { name = "tabulate" }, + { name = "tomli", specifier = ">=2.4.0" }, { name = "typing-extensions" }, { name = "validators", specifier = ">=0.28.3" }, ]