Repository navigation
Harden credential storage, diagnostics, and generated bundles - #860
Open
mconflitti-pbc wants to merge 1 commit into
Open
mconflitti-pbc wants to merge 1 commit into
mconflitti-pbc wants to merge 1 commit into
Conversation
Keep the deferred security changes separate from the additive agent workflows. Protect default credential writes with private atomic replacement, suppress sensitive auth diagnostics, and exclude CLI configuration from generated manifests and bundles. BREAKING CHANGE: saves require a writable parent, replace destination symlinks, and leave hardlink aliases unchanged. Diagnostics provide less detail and configuration files cannot be forced into bundles. Trusted custom openers and prepared archives keep existing behavior. Refs #859
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Reduce accidental credential exposure in saved files, authentication diagnostics, and generated deployment artifacts. This PR contains only the deferred
/tmp/rsconnect-security-hardening.patch; it builds on #859 and targets that feature branch so the review diff excludes resumable login and preflight implementation.Type of Change
The security protections intentionally change the edge cases described below. They are separate from the additive feature and are not a prerequisite for merging #859.
Approach
.rsconnect-pythonand the active CLI configuration directory, including explicit extras and resolved symlink aliases. Publishing from inside the configuration directory is rejected; static notebooks are rejected before execution. Tests cover Python, Node.js, notebook, Quarto, and manifest paths.Compatibility effects: credential updates require a writable parent directory; destination symlinks are replaced rather than followed, and hardlink aliases retain old contents. Human-readable diagnostics are less detailed. Configuration files can no longer be explicitly forced into generated deployment artifacts. Trusted custom I/O callbacks retain their existing direct-write behavior; prepared archives are uploaded unchanged.
Pending credentials remain plaintext accessible to the same operating-system user, root, and backups. This does not add token revocation or general credential scanning of application files or prepared archives. The POSIX-only boundary of the new workflows remains unchanged; no native Windows security helpers are introduced.
Automated Tests
Tests use temporary HOME/configuration directories and synthetic credentials. Subprocess integration tests exercise the actual CLI against local HTTP/OAuth fixtures, including pending-login files inside application content, errors that echo secrets, resumed token flows, and static-notebook rejection before execution or upload.
Directions for Reviewers
Review the diff against
horse-cockroach-43481eda, especiallymetadata.pyatomic replacement,http_support.py/oauth.pydiagnostic suppression, andbundle.pypath filtering. Confirm the intentional compatibility effects are acceptable before merging. The parent #859 preserves existing server aliases, public missing-account exception types, and direct Click callback behavior; those fixes remain in this branch.Independent Luna review — remaining findings:
errortext in exceptions (api.py,AbstractRemoteServer.handle_bad_response), so a server that echoes credentials can expose them through those messages.oauth.py,discover_oauth_metadata); credential-bearing query parameters in that URL can appear in errors.suppress_response_loggingandCookieJar.suppress_logs). There are no remaining repository callers, but external callers using those new keywords would receiveTypeError.These are recorded rather than extending the saved patch. This PR is targeted hardening, not a guarantee of complete credential suppression. The existing Quarto manifest function increases from CC 11 to 12 due to its file guard; new helpers meet the cyclomatic complexity limit.
This PR depends on #859. Retarget/rebase after the parent merges if the repository's merge workflow requires it.
Checklist
rsconnect-python-tests-at-nightworkflow in Connect against this feature branch.Live Connect and native Windows execution have not been validated locally.