Repository navigation
Harden credential storage, diagnostics, and generated bundles - #860
Merged
Merged
Conversation
|
mconflitti-pbc
force-pushed
the
horse-cockroach-43481eda
branch
from
October 14, 2026 19:57
738c467 to
a9c83e0
Compare
mconflitti-pbc
force-pushed
the
security-hardening-agent-workflows
branch
from
October 15, 2026 14:05
09af071 to
a807e84
Compare
mconflitti-pbc
added a commit
that referenced
this pull request
Oct 15, 2026
Keep credential writes private without replacing symlinks or hardlink aliases, or requiring a writable parent directory. Retain logging keywords and scoped OAuth diagnostics. Preserve registration deadline errors and cover compatibility regressions. Refs #860
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
joshyam-k
approved these changes
Oct 16, 2026
joshyam-k
left a comment
joshyam-k
left a comment
Contributor
There was a problem hiding this comment.
couple of small questions, but looks good!
| raise RSConnectException( | ||
| "Could not connect to %s - %s" % (self.token_endpoint(), response.exception), | ||
| "Could not connect to %s (%s)" | ||
| % (_redacted_uri_for_log(self.token_endpoint()), type(response.exception).__name__), |
Contributor
There was a problem hiding this comment.
maybe I'm just missing something, but why would this exception text contain ever contain a credential? Aren't these raised before the response is read?
Contributor
Author
There was a problem hiding this comment.
i had claude find all potential ways to leak and one test was able to get an exposed credential through the log so this will fix that edge case
| "node_modules/", | ||
| ] | ||
| directories_to_ignore = {Path(d) for d in directories_ignore_list} | ||
| _config_dir_names = {".rsconnect-python"} |
Contributor
There was a problem hiding this comment.
nice- this seems like a good safety thing to have.
| if response.exception: | ||
| raise RSConnectException("Could not connect to %s - %s" % (url, response.exception), cause=response.exception) | ||
| raise RSConnectException( | ||
| "Could not connect to %s (%s)" % (_redacted_uri_for_log(url), type(response.exception).__name__), |
Contributor
There was a problem hiding this comment.
same question here as above.
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
Keep credential writes private without replacing symlinks or hardlink aliases, or requiring a writable parent directory. Retain logging keywords and scoped OAuth diagnostics. Preserve registration deadline errors and cover compatibility regressions. Refs #860
mconflitti-pbc
force-pushed
the
security-hardening-agent-workflows
branch
from
October 16, 2026 17:29
ced5916 to
82e7867
Compare
mconflitti-pbc
enabled auto-merge (squash)
October 16, 2026 17:31
mconflitti-pbc
disabled auto-merge
October 16, 2026 17:31
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
2 participants
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.
Summary
Reduce accidental credential exposure in saved files, authentication diagnostics, and generated deployment artifacts while preserving existing login APIs and credential-file behavior. This PR contains targeted hardening on top of the merged agent workflows in #859.
Changes
0600, restrict existing files before truncating or writing, and repair permissions on unchanged files. Preserve in-place writes, flush/fsync, symlinks, hardlinks, writable files in read-only parent directories, fallback locations, and trusted custom openers..rsconnect-pythonpaths and the active CLI configuration directory from generated deployment file lists and manifest-based bundles, including explicit extras and symlink aliases. Check reserved names after qualifying paths against the content root and after resolving symlinks. Reject active or reserved configuration directories and their aliases as content roots while retaining safe symlinks.The changelog and command documentation are updated.
Compatibility and limits
Diagnostics intentionally provide less provider detail, and protected configuration paths cannot be forced into filtered artifacts. Prepared bundle archives are uploaded unchanged. Credential writes remain non-atomic, matching
main: an I/O failure after truncation can leave partial metadata. Atomic replacement is deferred because it changes link behavior and requires a writable parent directory.Existing general API error text and OAuth discovery error URLs can still expose echoed or URL-carried credentials. Filtering does not scan application file contents or identify hardlinks to credentials under ordinary filenames. Pending device codes remain on disk; token checkpoints are plaintext with file fallback or existing plaintext checkpoints. Keyring-backed retries require the same accessible keyring. If a reference-file write fails and the keyring backend then changes or becomes unavailable, recovery may re-poll an already-redeemed device code; keyring and file writes cannot be committed atomically. Abandoned pending entries have no background cleanup. No native Windows security helpers are added.
Validation
bundle.pyas the preceding revision, all in unchanged code.Tests use temporary HOME/configuration directories, synthetic credentials, an in-memory keyring, and local HTTP/OAuth fixtures. Native OS keyring behavior and manual production deployment were not checked.