Skip to content

fix: input validation gap in rsconnect add#758

Merged
joshyam-k merged 2 commits intomainfrom
add-cmd-fix
Feb 24, 2026
Merged

fix: input validation gap in rsconnect add#758
joshyam-k merged 2 commits intomainfrom
add-cmd-fix

Conversation

@joshyam-k
Copy link
Contributor

@joshyam-k joshyam-k commented Feb 24, 2026

Intent

Running rsconnect add with insufficient inputs (in this case only specifying the name) should result in a clear RSConnectException

fixes: #757

Type of Change

  • Bug Fix

Approach

The add command supports three credential pathways, each requiring a different set of options:

  1. Posit Connect — requires --server (and typically --api-key)
  2. shinyapps.io / Cloud — requires --account, --token, and --secret
  3. Snowflake (SPCS) — requires --server with a snowflakecomputing.app host (and optionally --snowflake-connection-name)

Adds an early check at the top of the add command. It raises an RSConnectException with a clear message if neither --server nor any of the shinyapps.io options are provided, before any downstream code can encounter None values

Note: click resolves env vars before calling the function, so this won't screw us up in cases where things are being specified as env vars instead of inputs

Automated Tests

Added a unit test test_add_name_only_missing_server_and_credentials

New behavior

uv run rsconnect add -n x
...
rsconnect.exception.RSConnectException: `rsconnect add` requires -s/--server (for Posit Connect) or -A/--account, -T/--token, and -S/--secret (for shinyapps.io).

Checklist

  • I have updated CHANGELOG.md to cover notable changes.
  • I have updated all related GitHub issues to reflect their current state.
  • I have run the rsconnect-python-tests-at-night workflow in Connect against this feature branch.

@github-actions
Copy link

github-actions bot commented Feb 24, 2026

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-02-24 22:55 UTC

@github-actions
Copy link

github-actions bot commented Feb 24, 2026

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
5431 4156 77% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
rsconnect/main.py 69% 🟢
TOTAL 69% 🟢

updated for commit: b473b7a by action🐍

set_verbosity(verbose)
output_params(ctx, locals().items())

if not server and not any([token, secret, account]):
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this belong inside validate_connection_options (below)?

Copy link
Contributor Author

@joshyam-k joshyam-k Feb 24, 2026

Choose a reason for hiding this comment

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

it probably could, but it would require a decent amount of additional complexity because validate_connection_options is used across commands. We can't just blindly reject name only inputs in it because commands like rsconnect deploy -n my-server are perfectly valid.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, that's sad. Ok.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Too much sadness in this pull request.


def test_add_name_only_missing_server_and_credentials(self):
"""Regression test: `rsconnect add -n x` should produce a validation error, not a TypeError."""
original_api_key_value = os.environ.pop("CONNECT_API_KEY", None)
Copy link
Contributor

Choose a reason for hiding this comment

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

It makes me sad that you have to do this env management in every test manually.

@nealrichardson
Copy link
Contributor

That test failure is weird (and surely unrelated?). I was seeing it in #742 as well (which made slightly more sense), but it makes me wonder if it's really saying that something is wrong, not that literally the model isn't right or whatever.

@joshyam-k
Copy link
Contributor Author

joshyam-k commented Feb 24, 2026

claude is recommending a package pin as a temporary fix. I'm going to try that before diving further into this

update: hmm that passed. maybe we just stumbled upon an issue with vetiver and pydantic>=2?

@joshyam-k
Copy link
Contributor Author

filed: rstudio/vetiver-python#241

@joshyam-k joshyam-k merged commit ee209aa into main Feb 24, 2026
23 checks passed
@joshyam-k joshyam-k deleted the add-cmd-fix branch February 24, 2026 22:54
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.

Missing validation of required inputs in rsconnect add

3 participants