Skip to content

Add secure GitHub token storage via Windows DPAPI#3296

Draft
jmcouffin wants to merge 8 commits into
developfrom
claude/secure-github-token-storage-WpVP3
Draft

Add secure GitHub token storage via Windows DPAPI#3296
jmcouffin wants to merge 8 commits into
developfrom
claude/secure-github-token-storage-WpVP3

Conversation

@jmcouffin
Copy link
Copy Markdown
Contributor

Introduces pyrevitlib/pyrevit/coreutils/credentials.py with public API
(get/set/delete_github_token) that encrypts tokens with ProtectedData
(CurrentUser scope) before writing to the pyRevit config file, so
plaintext never touches disk.

Includes a one-shot migrate_legacy_token() helper, called from
sessionmgr._perform_onsessionloadstart_ops() after upgrades run, that
re-encrypts any existing plaintext token and removes the plaintext entry.

https://claude.ai/code/session_012tJ8s3Vp1BUnePd7MFBtCA

Introduces pyrevitlib/pyrevit/coreutils/credentials.py with public API
(get/set/delete_github_token) that encrypts tokens with ProtectedData
(CurrentUser scope) before writing to the pyRevit config file, so
plaintext never touches disk.

Includes a one-shot migrate_legacy_token() helper, called from
sessionmgr._perform_onsessionloadstart_ops() after upgrades run, that
re-encrypts any existing plaintext token and removes the plaintext entry.

https://claude.ai/code/session_012tJ8s3Vp1BUnePd7MFBtCA
@jmcouffin jmcouffin marked this pull request as draft April 12, 2026 09:39
@pyrevitlabs pyrevitlabs deleted a comment from devloai Bot Apr 12, 2026
@jmcouffin jmcouffin requested a review from Copilot April 12, 2026 09:40
@jmcouffin
Copy link
Copy Markdown
Contributor Author

Possible implementation to solve: #3293
@Wurschdhaud

@jmcouffin
Copy link
Copy Markdown
Contributor Author

Again, 100% ai.
Needs to be stress tested

  • existing gh token in config gets deleted and added to keyring
  • can be reused
  • new one is store properly
  • and can be reused
  • can be deleted

This comment was marked as resolved.

- sessionmgr: wrap migrate_legacy_token() in try/except so a missing
  System.Security assembly or other import failure cannot abort startup

- migrate_legacy_token: verify the existing DPAPI blob is decryptable
  before deleting the legacy plaintext key; keep legacy if blob is corrupt

- get_github_token: fall back to legacy plaintext on decrypt failure
  rather than returning None, preserving access for existing setups

- set_github_token: delete the legacy plaintext key immediately after
  writing the encrypted value, not just at next startup migration

https://claude.ai/code/session_012tJ8s3Vp1BUnePd7MFBtCA

This comment was marked as resolved.

- Use System.Convert.ToBase64String/FromBase64String instead of
  Python base64 + bytes(bytearray(...)), which in IronPython 2 produces
  the repr of the bytearray rather than raw bytes, corrupting ciphertext

- Try clr.AddReference("System.Security.Cryptography.ProtectedData")
  first (required on .NET Core / Revit 2025+), falling back to
  "System.Security" for .NET Framework builds

https://claude.ai/code/session_012tJ8s3Vp1BUnePd7MFBtCA

This comment was marked as resolved.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jmcouffin jmcouffin marked this pull request as ready for review April 12, 2026 10:27
@Wurschdhaud
Copy link
Copy Markdown
Contributor

credentials.py has a non-ascii character but no encoding declared

fixing that locally leaves the token inside the pyRevit_config.ini, so the migration itself is not working
image

if i uninstall/reinstall it simply adds the plain text token back to the .ini

Looking at it, the AI never touched the config stuff in install_custom_extension of the extensionwindow smartbutton:

            # If token was provided, store it in config
            if token:
                temp_pkg.config.private_repo = True
                temp_pkg.config.token = token
                temp_pkg.config.username = 'oauth2'  # for backwards compat - drop later
                temp_pkg.config.password = token  # for backwards compat - drop later
                user_config.save_changes()  # i don't like it - drop this later

            extpkgs.install(temp_pkg, dest_path)

@Wurschdhaud
Copy link
Copy Markdown
Contributor

Also updater.py must be updated so the update button can work. it currently retrieves credentials here:

def _get_extension_credentials(repo_info):
    try:
        repo_config = user_config.get_section(repo_info.name)
        if repo_config.private_repo:
            return repo_config.username, repo_config.password
        return None, None
    except Exception:
        return None, None

- credentials.py: add UTF-8 encoding declaration (fixes non-ASCII
  em-dash SyntaxError in IronPython 2); expand migrate_legacy_token()
  to scan per-extension config sections for legacy plaintext
  token/username/password keys, not just [github].token

- Extensions.smartbutton/script.py: both install paths (catalog and
  custom URL) now call credentials.set_github_token(token) instead of
  writing plaintext token/username/password into the extension config;
  only private_repo=True is persisted per-extension

- updater.py: _get_extension_credentials() now reads the global
  DPAPI-encrypted token via credentials.get_github_token() for private
  repos, with a fallback to legacy per-extension plaintext password for
  installations not yet migrated

https://claude.ai/code/session_012tJ8s3Vp1BUnePd7MFBtCA
@Wurschdhaud
Copy link
Copy Markdown
Contributor

Latest commit has:
File "C:\XXX\pyRevit\pyrevitlib\pyrevit\coreutils\git.py", line 164, in _process_git_error
raise PyRevitException(exception_msg)
PyRevitException: remote authentication required but no callback set

Traceback:
File "C:\XXX\pyRevit\extensions\pyRevitCore.extension\pyRevit.tab\pyRevit.panel\Extensions.smartbutton\script.py", line 581, in install_custom_extension
extpkgs.install(temp_pkg, dest_path)

but at least adds sth to the config:

image

Whatever this is, haven't read the code in detail:

  • it probably should be on a per extension basis, not a simple "github" entry
  • it should not be called github at all - gitlab, gitbucket, etc. do exist and have to be supported

Comment on lines +578 to +579
from pyrevit.coreutils import credentials
credentials.set_github_token(token)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not simply coreutils.credentials.set_github_token(token) ? coreutils is imported at the top

self.selected_pkg.ext_pkg.config.private_repo = True
self.selected_pkg.ext_pkg.config.token = token
from pyrevit.coreutils import credentials
credentials.set_github_token(token)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not simply coreutils.credentials.set_github_token(token) ? coreutils is imported at the top

# Fall back to the legacy per-extension plaintext password if present
# (covers installations not yet migrated).
from pyrevit.coreutils import credentials
token = credentials.get_github_token()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not simply coreutils.credentials.set_github_token(token) ? coreutils is imported at the top

@jmcouffin
Copy link
Copy Markdown
Contributor Author

@Wurschdhaud do not waste your time on this, mostly AI slop.
I just tried from my phone without even reading the results.

@jmcouffin jmcouffin marked this pull request as draft April 12, 2026 15:20
Wurschdhaud and others added 2 commits April 28, 2026 19:32
Root cause of the auth error: set_token() wrote to [github] config section
but _install_extpkg read extpkg.config.token (per-extension section), so
credentials were never passed to git_clone.

- credentials.py: redesign API as get/set/delete_token(url) keyed by hostname
  ([git_credentials] section, keys like github_com_token_dpapi). Supports
  GitHub, GitLab, Gitbucket, self-hosted instances. Migration from old
  [github] section and per-extension plaintext tokens.
- extpackages.py: _install_extpkg now calls credentials.get_token(url),
  which is the actual fix that wires the stored token into git_clone.
- script.py: module-level credentials import; both install paths call
  set_token(url, token) with the real git URL.
- updater.py: module-level credentials import; _get_extension_credentials
  extracts the remote URL and calls get_token(remote_url).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each extension stores its encrypted token inside its own config section
(token_dpapi key, alongside disabled/private_repo). Two private extensions
from the same host get independent tokens; any host platform is supported.

- credentials.py: get/set/delete_token(section_name) where section_name is
  the extension folder name. Removes the old global [github]/[git_credentials]
  sections on first startup (tokens non-functional anyway); migrates legacy
  per-extension plaintext token/password keys in-place.
- extpackages.py: credentials.get_token(extpkg.config_section_name)
- script.py: credentials.set_token(extpkg.config_section_name / temp_pkg.config_section_name, token)
- updater.py: credentials.get_token(repo_info.name) -- folder name matches
  config_section_name for installed extensions

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Wurschdhaud
Copy link
Copy Markdown
Contributor

Added some additional AI slop - now it works. Try to break it

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