[Core] Write session files atomically so a partial write cannot destroy them

[om singhal (Om-singhaI)]

Related command

Not command specific. azure/cli/core/_session.py backs ~/.azure/azureProfile.json, commandIndex.json and the rest.

Description

Fixes #9427.

save() opens the target with 'w', so the file is truncated in place and is empty or half a document while the write runs. A process reading it in that window fails to parse it, and load() then overwrites it with defaults. The file is also lost if the write itself raises partway, with no second process involved.

save() now writes a temporary file in the same directory and renames it over the target, so a reader sees either the old document or the new one.

The rename follows a symlink rather than replacing it, carries over the existing file's permissions, and falls back to the in place write when the config directory is not writable, since that still works today.

It does not serialize concurrent writers. Two saves can still overwrite each other, cleanly rather than corruptly, which is #14070.

Testing Guide

Adds test_session.py with eight tests covering the atomic replace, a save that raises partway, permissions, symlinks and a read only directory.

python -m unittest azure.cli.core.tests.test_session
Ran 8 tests in 0.004s
OK

History Notes

[Core] Fix ~/.azure files being corrupted or emptied when a save is interrupted or another process reads the file mid write

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[microsoft-github-policy-service]

Thank you for your contribution om singhal (@Om-singhaI)! We will review the pull request and get back to you soon.

[Copilot]

🟡 Changes recommended

The critical fallback can truncate files after non-permission errors, and test temporary directories need cleanup.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates Azure CLI session persistence to write JSON files atomically, preserving permissions and symlink targets.

Changes:

• Writes through same-directory temporary files before replacement.
• Adds fallback behavior and failure cleanup.
• Adds tests for atomic saves and edge cases.

File summaries

File	Summary
src/azure-cli-core/azure/cli/core/_session.py	Implements atomic session writes. Critical (3 votes): fallback catches all OSError values and may reintroduce data loss.
src/azure-cli-core/azure/cli/core/tests/test_session.py	Adds session persistence tests. Nit (1 vote): temporary directories are not cleaned up.

Review details

Suppressed comments (1)

src/azure-cli-core/azure/cli/core/tests/test_session.py:19

• Each test creates a new mkdtemp() directory, but this fixture is never removed, so repeated runs leave eight directories behind in the system temp directory. Core tests using the same pattern clean up in tearDown (for example, test_extension.py:79-94 and test_azlogging.py:18-23); register an addCleanup/TemporaryDirectory cleanup here.

        self.dir = tempfile.mkdtemp()
        self.filename = os.path.join(self.dir, 'test.json')

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Yong Zhang (yonzhan)]

Core

[Copilot]

🔵 Needs a closer look

The test suite has unresolved Windows compatibility and read-only fallback coverage issues.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

src/azure-cli-core/azure/cli/core/tests/test_session.py:68

• This test asserts POSIX mode bits, but it is not skipped on Windows. os.chmod on Windows does not preserve arbitrary 0o644 permissions, so stat.S_IMODE(os.stat(...).st_mode) == 0o644 can fail on the Windows test runner; the existing permission tests in azure-cli-core/azure/cli/core/tests/test_azlogging.py:41 skip Windows for this reason. Skip this test on os.name == 'nt' or assert only platform-independent behavior.

This issue also appears on line 76 of the same file.

src/azure-cli-core/azure/cli/core/tests/test_session.py:76

• os.symlink exists on Windows even when creating one requires the symlink privilege/Developer Mode, so checking only hasattr does not prevent PermissionError on standard Windows runners. The repository skips its other symlink test on Windows (azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py:497); apply the same platform guard here or convert a permission failure into a skipped test.

    @unittest.skipUnless(hasattr(os, 'symlink'), 'requires symlink support')

src/azure-cli-core/azure/cli/core/tests/test_session.py:123

• Mocking mkstemp to return EROFS leaves the destination on the writable test filesystem, so the fallback open(..., 'w') succeeds. On a real read-only filesystem that open would also return EROFS, so this test does not cover the claimed scenario and can give false confidence. Test EACCES/EPERM for a non-writable directory with an existing writable file, or remove EROFS from the fallback contract.

        with mock.patch('tempfile.mkstemp', side_effect=OSError(errno.EROFS, 'Read-only file system')):
            self._session({'a': 1}).save()

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Fix fallback serialization safety and make the permission test reliably exercise the fallback path.

Review details

Suppressed comments (2)

src/azure-cli-core/azure/cli/core/_session.py:79

• When mkstemp fails with EACCES/EPERM, this fallback opens the live file with 'w' and then serializes directly into it. If json.dump raises after emitting a prefix (for example, because a later value is not JSON-serializable), the existing session file is left truncated, so the partial-save protection is lost on the read-only-directory path. Serialize to a string before opening the target; write failures can still be unavoidable in this fallback, but serialization errors should not destroy the old file.

            with open(self.filename, 'w', encoding=self._encoding) as f:
                json.dump(self.data, f)

src/azure-cli-core/azure/cli/core/tests/test_session.py:102

• This test relies on chmod(0o500) to make mkstemp fail, so a root or otherwise privileged test process can still create the temporary file and pass through the atomic path without exercising the permission-denied fallback. Mock mkstemp to raise EACCES while retaining the writable existing file, or otherwise assert that the fallback path was taken.

        os.chmod(self.dir, 0o500)
        try:
            self._session({'a': 1}).save()

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite