SK-3039:Fix missing roles & ctx in bearerToken.

[yaswanth-pula-skyflow]

SK-3039: Fix missing roles/ctx in bearer tokens and harden request/config input validation

Problem

• roles and context set under credentials in add_vault_config() never reached the generated bearer token. VaultClient.get_bearer_token() read roles/ctx from the top-level config, but both are only accepted nested under credentials — top-level keys are rejected by validate_keys, so the lookup always resolved to None: no error, just a token with no ctx claim and no role: scope, on first generation and every auto-refresh.
• context was validated as str only, rejecting a dict/JSON object at build() even though the token-generation engine already supports it.
• Several request/config validators accepted the wrong Python type and either crashed with a raw, uncaught exception or silently produced wrong behaviour instead of raising a clean SkyflowError: dict tokens on insert, bare-string items in detokenize data, non-string elements in roles, non-list ids on delete, and non-string skyflow_id/table/column_name/file_name on update and file-upload.
• validate_update_connection_config dropped connection_id from validate_credentials, so credential errors on the update path lost config context that the add path already included.

Changes

Bearer token roles/ctx

• VaultClient.get_bearer_token() now builds token options from the resolved credentials dict instead of the top-level config, and calls the shared validate_token_options directly — so a directly-constructed VaultClient can no longer skip roles/context validation. Keys are omitted when unset rather than passed as None.
• Fixes both first generation and every auto-refresh, for path and credentials_string, config-level and common credentials, and connection configs.
• credentials.context now accepts a str or a dict. Dict keys are validated at config time (^[a-zA-Z0-9_]+$), so an invalid key fails at build() instead of on the first API call. Empty dict is rejected as an empty context, matching existing empty-string behaviour.
• Fixed swapped empty/invalid error messages for roles: a non-list now reports "Specify roles as an array" instead of "Specify at least one role".
• roles: [] now raises EMPTY_ROLES instead of silently producing an unscoped token. Each element of roles must now be a non-empty string, or validation raises — previously a non-string role was silently stringified into the OAuth scope.
• validate_update_connection_config now passes connection_id through to validate_credentials, so its error messages include the connection id like the add path already does.

Insert / detokenize type safety

• InsertRequest(tokens=...) with a non-list/non-dict-of-dicts value now raises INVALID_TYPE_OF_DATA_IN_INSERT instead of crashing in the diagnostic logging loop.
• DetokenizeRequest(data=...) with bare strings now always raises INVALID_TOKENS_LIST_VALUE, instead of only crashing when a string happened to contain "token" as a substring.

Delete / update / file-upload validation

• DeleteRequest(ids=...) with a non-list value now raises INVALID_IDS_TYPE.
• UpdateRequest and FileUploadRequest now type-check skyflow_id (new INVALID_SKYFLOW_ID_TYPE message), and FileUploadRequest type-checks table, column_name, and file_name, raising the existing corresponding error instead of crashing on .strip().

Behaviour change

roles: [] and non-string roles elements previously passed validation and produced an unscoped or corrupted token; both now raise. Everything else is either a crash fix or a message correction — the accept/reject boundary for already-valid input is unchanged.

roles and context remain optional; validation only runs when the key is present.

Tests

• tests/vault/client/test__client.py: string and dict context plus roles forwarded on the path and credentials-string flows; options omitted when unset; top-level config keys explicitly ignored; token_uri coexistence; refresh path verified directly and end-to-end through initialize_client_configuration().
• tests/utils/validations/test__validations.py: string/dict context accepted, empty dict, invalid ctx key, invalid types, config-scoped message variant, end-to-end validate_vault_config, the three roles cases plus non-string role elements, dict tokens on insert, bare-string detokenize data, non-list delete ids, non-string skyflow_id/table/column_name/file_name on update and file-upload, and connection_id included in validate_update_connection_config error messages.
• tests/client/test_skyflow.py: end-to-end builder().add_vault_config(...).build() asserting the options handed to the token engine.
• Removed the stale top-level roles/ctx from the client test fixture — that shape is rejected by real validation.

[Copilot]

Pull request overview

This PR fixes propagation and validation of credentials.roles and credentials.context so they reliably reach the service-account bearer token generation (including refresh), and expands credentials.context to accept dict/JSON objects with key validation.

Changes:

• Update VaultClient.get_bearer_token() to build token-engine options from the resolved credentials dict (not top-level config) and omit unset options.
• Update credential validation to (a) reject roles: [], (b) accept context as str or dict, and (c) validate dict context keys early.
• Add/adjust unit and end-to-end tests covering roles/context forwarding and the new validation behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/vault/client/test__client.py	Adds coverage for roles/context forwarding into token generation options (including refresh) and verifies top-level config keys are ignored.
tests/utils/validations/test__validations.py	Adds validation coverage for dict context, invalid keys/types, and the updated roles empty/type error behavior.
tests/client/test_skyflow.py	Adds end-to-end tests ensuring builder-configured roles/context reach the token engine.
skyflow/vault/client/client.py	Changes bearer-token option construction to read from credentials and omit unset values.
skyflow/utils/validations/_validations.py	Updates validation for roles empty list, and allows dict context with key validation via the token-engine helper.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

tests/client/test_skyflow.py:461

• Same issue as above: @patch("skyflow.vault.client.client.Skyflow") targets a non-existent symbol and will raise at test import/runtime. Remove it and update the test signature accordingly.

    @patch("skyflow.vault.client.client.Skyflow")
    @patch("skyflow.vault.client.client.generate_bearer_token_from_creds", return_value=("token", "bearer"))
    def test_dict_context_reaches_token_engine(self, mock_gen, _mock_api):

skyflow/utils/validations/_validations.py:533

• The request.tokens type validation only runs when request.tokens is truthy. This means invalid-but-falsy values (e.g. {}) bypass validation and can still flow into insert body building, potentially causing subtle behavior differences. If the intent is to validate whenever the caller provides tokens, gate on is not None instead of truthiness, and avoid the redundant or not request.tokens check (it can never be true inside if request.tokens:).

    if request.tokens:
        if not isinstance(request.tokens, list) or not request.tokens or not all(
                isinstance(t, dict) for t in request.tokens):
            log_error_log(SkyflowMessages.ErrorLogs.EMPTY_TOKENS.value.format(RequestOperation.INSERT), logger=logger)
            raise SkyflowError(SkyflowMessages.Error.INVALID_TYPE_OF_DATA_IN_INSERT.value, invalid_input_error_code)

skyflow/utils/validations/_validations.py:573

• validate_delete_request checks emptiness before validating type. For non-list falsy values (e.g. ids="" or ids=()), this raises EMPTY_RECORD_IDS_IN_DELETE instead of the intended INVALID_IDS_TYPE. To make the new type-safety guarantee consistent, validate ids type first (keeping a dedicated None check), then validate emptiness for lists.

    if not request.ids:
        log_error_log(SkyflowMessages.ErrorLogs.EMPTY_IDS.value.format(RequestOperation.DELETE), logger=logger)
        raise SkyflowError(SkyflowMessages.Error.EMPTY_RECORD_IDS_IN_DELETE.value, invalid_input_error_code)

    if not isinstance(request.ids, list):