Skip to content

Harden credential store file permissions - #1526

Merged
mishrarish merged 3 commits into
masterfrom
users/bhatibhavesh/filePermissions
Oct 16, 2026
Merged

mishrarish merged 3 commits into
masterfrom
users/bhatibhavesh/filePermissions

Conversation

@bhaveshbhati

@bhaveshbhati Bhavesh Bhati (bhaveshbhati) commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Please make sure the code is following contribution guidelines in CONTRIBUTING.md

  • : This PR has a corresponding issue open in the Repository.
  • : Approach is signed off on the issue.

Summary

PAT-file permission hardening affects only the Linux file-fallback path. Configuration-directory creation is cross-platform, with explicit  0700  enforcement limited to POSIX systems.

Changes

  • Creates a new Azure DevOps configuration directory with permissions 0700.
  • Preserves permissions on an existing configuration directory, including directories selected through AZURE_DEVOPS_EXT_CONFIG_DIR.
  • Creates new personalAccessTokens files with permissions 0600.
  • Changes existing PAT files to 0600 before truncating or rewriting them.
  • Preserves existing credentials if file-permission hardening fails.
  • Keeps file-based credential storage as a Linux-only fallback when keyring storage is unavailable.
  • Adds unit tests covering new and existing directory permissions, restrictive umasks, PAT-file permissions, and permission-hardening failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ea18fbf5-c89f-4955-bd1d-4dfb4c44cdb7
@triptijain2112

Copy link
Copy Markdown
Collaborator

Bhavesh Bhati (@bhaveshbhati) Please update PR description related to changes made

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The required corresponding issue and documented approach approval are missing from the PR metadata.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Hardens local Azure DevOps credential storage permissions on POSIX systems.

Changes:

  • Restricts configuration directories to 0700.
  • Restricts PAT files to 0600.
  • Adds permission and credential-persistence tests.
File Description
dev/​common/​config.py Creates and hardens the config directory.
dev/​common/​credential_store.py Secures PAT writes and updates warnings.
tests/​latest/​common/​test_config.py Tests directory permissions.
tests/​latest/​common/​test_credential_store.py Tests PAT permissions and persistence.

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

_UNSET = object()


def _ensure_private_config_dir(config_dir):
Bhavesh Bhati added 2 commits October 14, 2026 13:55
Harden existing PAT files before truncating them and add regression coverage for permission failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 43a1d6f4-47fa-405f-a4dc-fb6b93b44ae8
Update the organization list only after credential deletion succeeds, so permission-hardening failures leave the stored PAT discoverable for a later retry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 43a1d6f4-47fa-405f-a4dc-fb6b93b44ae8
@mishrarish
mishrarish merged commit 429a129 into master Oct 16, 2026
21 checks passed
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.

4 participants