Skip to content

DEBUG: limit workflow #2390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 24 commits into
base: dev/aio-connector
Choose a base branch
from
Draft

DEBUG: limit workflow #2390

wants to merge 24 commits into from

Conversation

sfc-gh-pczajka
Copy link
Collaborator

DO NOT MERGE, this is a workflow debug branch

@sfc-gh-pczajka sfc-gh-pczajka force-pushed the pczajka-debug branch 3 times, most recently from 355bc1c to 5ec89c3 Compare July 4, 2025 14:27
@sfc-gh-pczajka sfc-gh-pczajka force-pushed the cherrypicks-to-aio-connector-part1 branch from 5531b8e to 5168b00 Compare July 4, 2025 14:45
@sfc-gh-pczajka sfc-gh-pczajka force-pushed the pczajka-debug branch 2 times, most recently from 042c5ab to b2f1bb9 Compare July 7, 2025 14:26
@sfc-gh-pczajka sfc-gh-pczajka force-pushed the cherrypicks-to-aio-connector-part1 branch from 5168b00 to 9a188b1 Compare July 7, 2025 14:28
@sfc-gh-pczajka sfc-gh-pczajka force-pushed the cherrypicks-to-aio-connector-part1 branch 2 times, most recently from c37c333 to f0d9e29 Compare July 7, 2025 15:54
@sfc-gh-pczajka sfc-gh-pczajka force-pushed the pczajka-debug branch 2 times, most recently from 098c4d2 to b2719d0 Compare July 8, 2025 10:23
@sfc-gh-pczajka sfc-gh-pczajka marked this pull request as draft July 8, 2025 10:23
@sfc-gh-pczajka sfc-gh-pczajka force-pushed the cherrypicks-to-aio-connector-part1 branch from f0d9e29 to 449d668 Compare July 9, 2025 14:39
Base automatically changed from cherrypicks-to-aio-connector-part1 to dev/aio-connector July 14, 2025 12:26
@sfc-gh-pczajka sfc-gh-pczajka force-pushed the pczajka-debug branch 4 times, most recently from d56b698 to b919832 Compare July 21, 2025 12:57
@sfc-gh-pczajka sfc-gh-pczajka changed the base branch from dev/aio-connector to main July 21, 2025 12:58
Comment on lines 149 to 152
{
k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******"
for (k, v) in body["data"].items()
},

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.

Copilot Autofix

AI 21 days ago

To fix the problem, we should ensure that sensitive fields such as "PASSCODE", "password", and any other known sensitive keys are always masked in logs, regardless of their presence in any whitelist. This can be achieved by defining a set of sensitive keys and updating the dictionary comprehension in the logger statement to mask these keys. The fix should be applied in the authenticate method of the Auth class, specifically at the logging statement on lines 147–153. No changes to existing functionality are required, only the logging output is made safer. We will define a set of sensitive keys within the method (or at the top of the file if appropriate) and update the logging statement accordingly.


Suggested changeset 1
src/snowflake/connector/aio/auth/_auth.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/snowflake/connector/aio/auth/_auth.py b/src/snowflake/connector/aio/auth/_auth.py
--- a/src/snowflake/connector/aio/auth/_auth.py
+++ b/src/snowflake/connector/aio/auth/_auth.py
@@ -146,2 +146,4 @@
 
+        # Always mask sensitive fields in logs, regardless of whitelist
+        SENSITIVE_KEYS = {"PASSCODE", "password", "pwd", "secret", "token"}
         logger.debug(
@@ -149,3 +151,3 @@
             {
-                k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******"
+                k: "******" if k.upper() in SENSITIVE_KEYS else (v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******")
                 for (k, v) in body["data"].items()
EOF
@@ -146,2 +146,4 @@

# Always mask sensitive fields in logs, regardless of whitelist
SENSITIVE_KEYS = {"PASSCODE", "password", "pwd", "secret", "token"}
logger.debug(
@@ -149,3 +151,3 @@
{
k: v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******"
k: "******" if k.upper() in SENSITIVE_KEYS else (v if k in AUTHENTICATION_REQUEST_KEY_WHITELIST else "******")
for (k, v) in body["data"].items()
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines 867 to 673
c[0][1].startswith(
"https://test-account.snowflakecomputing.com:443"
)

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization

The string [https://test-account.snowflakecomputing.com:443](1) may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 21 days ago

To fix the issue, we should replace the startswith check with a more robust method that parses the URL and validates its components. Specifically, we can use Python's urllib.parse module to extract the hostname and scheme from the URL and ensure they match the expected values. This approach eliminates the risk of substring matching errors and ensures that the URL is validated correctly.

The changes will involve:

  1. Importing the urlparse function from urllib.parse.
  2. Replacing the startswith check with a parsed URL validation that ensures the scheme is https, the hostname matches the expected value, and the port is correct.

Suggested changeset 1
test/integ/aio/test_connection_async.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/test/integ/aio/test_connection_async.py b/test/integ/aio/test_connection_async.py
--- a/test/integ/aio/test_connection_async.py
+++ b/test/integ/aio/test_connection_async.py
@@ -840,5 +840,8 @@
             )  # Skip tear down, there's only a mocked rest api
+            from urllib.parse import urlparse
             assert any(
                 [
-                    c[0][1].startswith("https://test-host:443")
+                    urlparse(c[0][1]).scheme == "https"
+                    and urlparse(c[0][1]).hostname == "test-host"
+                    and urlparse(c[0][1]).port == 443
                     for c in mocked_fetch.call_args_list
@@ -864,7 +867,8 @@
             )  # Skip tear down, there's only a mocked rest api
+            from urllib.parse import urlparse
             assert any(
                 [
-                    c[0][1].startswith(
-                        "https://test-account.snowflakecomputing.com:443"
-                    )
+                    urlparse(c[0][1]).scheme == "https"
+                    and urlparse(c[0][1]).hostname == "test-account.snowflakecomputing.com"
+                    and urlparse(c[0][1]).port == 443
                     for c in mocked_fetch.call_args_list
EOF
@@ -840,5 +840,8 @@
) # Skip tear down, there's only a mocked rest api
from urllib.parse import urlparse
assert any(
[
c[0][1].startswith("https://test-host:443")
urlparse(c[0][1]).scheme == "https"
and urlparse(c[0][1]).hostname == "test-host"
and urlparse(c[0][1]).port == 443
for c in mocked_fetch.call_args_list
@@ -864,7 +867,8 @@
) # Skip tear down, there's only a mocked rest api
from urllib.parse import urlparse
assert any(
[
c[0][1].startswith(
"https://test-account.snowflakecomputing.com:443"
)
urlparse(c[0][1]).scheme == "https"
and urlparse(c[0][1]).hostname == "test-account.snowflakecomputing.com"
and urlparse(c[0][1]).port == 443
for c in mocked_fetch.call_args_list
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Comment on lines 363 to 459
name: Test asyncio ${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
needs: build
runs-on: ${{ matrix.os.image_name }}
strategy:
fail-fast: false
matrix:
os:
- image_name: ubuntu-latest
download_name: linux
python-version: [3.8]
cloud-provider: [aws]
download_name: manylinux_x86_64
- image_name: macos-latest
download_name: macosx_x86_64
- image_name: windows-latest
download_name: win_amd64
python-version: ["3.10", "3.11", "3.12"]
cloud-provider: [aws, azure, gcp]
steps:
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
- name: Display Python version
run: python -c "import sys; print(sys.version)"
- name: Setup parameters file
shell: bash
env:
PARAMETERS_SECRET: ${{ secrets.PARAMETERS_SECRET }}
run: |
gpg --quiet --batch --yes --decrypt --passphrase="$PARAMETERS_SECRET" \
.github/workflows/parameters/public/parameters_${{ matrix.cloud-provider }}.py.gpg > test/parameters.py
- name: Download wheel(s)
uses: actions/download-artifact@v4
with:
name: ${{ matrix.os.download_name }}_py${{ matrix.python-version }}
path: dist
- name: Show wheels downloaded
run: ls -lh dist
shell: bash
- name: Upgrade setuptools, pip and wheel
run: python -m pip install -U setuptools pip wheel
- name: Install tox
run: python -m pip install tox>=4
- name: Run tests
run: python -m tox run -e olddriver
run: python -m tox run -e aio
env:
PYTHON_VERSION: ${{ matrix.python-version }}
cloud_provider: ${{ matrix.cloud-provider }}
PYTEST_ADDOPTS: --color=yes --tb=short
TOX_PARALLEL_NO_SPINNER: 1
shell: bash
- name: Combine coverages
run: python -m tox run -e coverage --skip-missing-interpreters false
shell: bash
- uses: actions/upload-artifact@v4
with:
name: coverage_aio_${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
path: |
.tox/.coverage
.tox/coverage.xml

test-noarrowextension:
name: No Arrow Extension Test ${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
needs: lint
test-unsupporeted-aio:

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI 13 days ago

To fix the problem, you should add an explicit permissions block to the workflow file .github/workflows/build_test.yml. The best way to do this is to add the block at the top level of the workflow, so it applies to all jobs unless overridden. Since the jobs only need to read repository contents (for checkout and artifact download), the minimal required permission is contents: read. This change should be made immediately after the name: and before the on: block (i.e., after line 1 and before line 3). No additional imports or definitions are needed.


Suggested changeset 1
.github/workflows/build_test.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml
--- a/.github/workflows/build_test.yml
+++ b/.github/workflows/build_test.yml
@@ -1,2 +1,4 @@
 name: Build and Test
+permissions:
+  contents: read
 
EOF
@@ -1,2 +1,4 @@
name: Build and Test
permissions:
contents: read

Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Comment on lines 424 to 539
# name: Combine coverage
# needs: [lint, test, test-fips, test-lambda, test-aio]
# runs-on: ubuntu-latest
# steps:
# - uses: actions/checkout@v4
# - uses: actions/download-artifact@v4
# with:
# path: artifacts
# - name: Set up Python
# uses: actions/setup-python@v4
# with:
# python-version: '3.9'
# - name: Display Python version
# run: python -c "import sys; print(sys.version)"
# - name: Upgrade setuptools and pip
# run: python -m pip install -U setuptools pip wheel
# - name: Install tox
# run: python -m pip install tox>=4
# - name: Collect all coverages to one dir
# run: |
# python -c '
# from pathlib import Path
# import shutil

combine-coverage:
if: ${{ success() || failure() }}
name: Combine coverage
needs: [lint, test, test-fips, test-lambda]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/download-artifact@v4
with:
path: artifacts
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.8'
- name: Display Python version
run: python -c "import sys; print(sys.version)"
- name: Upgrade setuptools and pip
run: python -m pip install -U setuptools pip wheel
- name: Install tox
run: python -m pip install tox>=4
- name: Collect all coverages to one dir
run: |
python -c '
from pathlib import Path
import shutil

src_dir = Path("artifacts")
dst_dir = Path(".") / ".tox"
dst_dir.mkdir()
for src_file in src_dir.glob("*/.coverage"):
dst_file = dst_dir / ".coverage.{}".format(src_file.parent.name[9:])
print("{} copy to {}".format(src_file, dst_file))
shutil.copy(str(src_file), str(dst_file))'
- name: Combine coverages
run: python -m tox run -e coverage
- name: Publish html coverage
uses: actions/upload-artifact@v4
with:
include-hidden-files: true
name: overall_cov_html
path: .tox/htmlcov
- name: Publish xml coverage
uses: actions/upload-artifact@v4
with:
include-hidden-files: true
name: overall_cov_xml
path: .tox/coverage.xml
- uses: codecov/codecov-action@v4
with:
files: .tox/coverage.xml
token: ${{ secrets.CODECOV_TOKEN }}
# src_dir = Path("artifacts")
# dst_dir = Path(".") / ".tox"
# dst_dir.mkdir()
# for src_file in src_dir.glob("*/.coverage"):
# dst_file = dst_dir / ".coverage.{}".format(src_file.parent.name[9:])
# print("{} copy to {}".format(src_file, dst_file))
# shutil.copy(str(src_file), str(dst_file))'
# - name: Combine coverages
# run: python -m tox run -e coverage
# - name: Publish html coverage
# uses: actions/upload-artifact@v4
# with:
# include-hidden-files: true
# name: overall_cov_html
# path: .tox/htmlcov
# - name: Publish xml coverage
# uses: actions/upload-artifact@v4
# with:
# include-hidden-files: true
# name: overall_cov_xml
# path: .tox/coverage.xml
# - uses: codecov/codecov-action@v4
# with:
# files: .tox/coverage.xml
# token: ${{ secrets.CODECOV_TOKEN }}

Check warning

Code scanning / CodeQL

Workflow does not contain permissions

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}

Copilot Autofix

AI 13 days ago

To fix the problem, we should add a permissions block to the workflow or to each job that does not already have one. The minimal starting point is contents: read, which allows the workflow to read repository contents but not write to them. This should be added at the root level of the workflow (top-level, just after the name and before on), so it applies to all jobs unless overridden. No additional imports or definitions are needed; this is a YAML configuration change.

Suggested changeset 1
.github/workflows/build_test.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml
--- a/.github/workflows/build_test.yml
+++ b/.github/workflows/build_test.yml
@@ -2,2 +2,5 @@
 
+permissions:
+  contents: read
+
 on:
EOF
@@ -2,2 +2,5 @@

permissions:
contents: read

on:
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
@sfc-gh-pczajka sfc-gh-pczajka changed the base branch from main to cherrypicks-to-aio-connector-part5 August 6, 2025 11:23
@sfc-gh-pczajka sfc-gh-pczajka changed the base branch from cherrypicks-to-aio-connector-part5 to dev/aio-connector August 6, 2025 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants