fix(security): restrict permissions of auth token file to 0600#67
fix(security): restrict permissions of auth token file to 0600#67refraction-ray wants to merge 6 commits intomasterfrom
Conversation
- Use `os.open` with `0o600` when creating the token file to ensure it is not readable by others. - Explicitly `os.chmod` the file to `0o600` to secure existing files. - Add `tests/test_auth_security.py` to verify the fix and ensure no regressions. - Mock `os.path.expanduser` in tests to prevent side effects on user's home directory. Co-authored-by: refraction-ray <35157286+refraction-ray@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
please remove the separate test file |
I have removed |
…te test file - Removed `tests/test_auth_security.py`. - Moved `test_token_file_permissions` to `tests/test_cloud.py`. - Refactored `tests/test_cloud.py` to use per-test skipping logic instead of module-level skip, allowing unit tests like `test_token_file_permissions` to run without `TC_CLOUD_TEST` env var. Co-authored-by: refraction-ray <35157286+refraction-ray@users.noreply.github.com>
refraction-ray
left a comment
refraction-ray
left a comment
There was a problem hiding this comment.
pay attention to pass check_all
| if "TC_CLOUD_TEST" not in os.environ: | ||
| pytest.skip(allow_module_level=True) | ||
| # skip on CI due to no token | ||
| skip_cloud = pytest.mark.skipif("TC_CLOUD_TEST" not in os.environ, reason="no token") |
There was a problem hiding this comment.
could you better to organize the test in this file into two classes, one with skip_cloud, the other one without?
There was a problem hiding this comment.
Done. I have reorganized tests/test_cloud.py into two classes: TestCloud (decorated with skip_cloud) and TestCloudAuth (not skipped). This allows the authentication unit test to run independently of the integration tests.
- Created `TestCloud` class decorated with `@skip_cloud` for integration tests requiring a token. - Created `TestCloudAuth` class (not skipped) for the `test_token_file_permissions` unit test. - This ensures the security test runs in all environments while keeping integration tests skipped when no token is present. Co-authored-by: refraction-ray <35157286+refraction-ray@users.noreply.github.com>
|
GitHub action still fails, make it pass |
I identified that the new test |
- The `TestCloudAuth` class was modifying the global `saved_token` variable in `tensorcircuit.cloud.apis` without restoring it. - This caused subsequent integration tests (which run when `TC_CLOUD_TEST` is set) to fail because they relied on the original token state. - Added a `finally` block to `test_token_file_permissions` to restore `apis.saved_token` to its original value. Co-authored-by: refraction-ray <35157286+refraction-ray@users.noreply.github.com>
refraction-ray
left a comment
refraction-ray
left a comment
There was a problem hiding this comment.
pass bash check_all.sh, it is necessary
| data = json.load(f) | ||
| assert data is not None | ||
| finally: | ||
| # Restore original saved_token |
There was a problem hiding this comment.
dont think loudly like this, no one want to see you thought as comment
There was a problem hiding this comment.
Apologies for the clutter. I have removed the internal monologue comments from the test file.
- Removed "thinking out loud" comments from the teardown block in `TestCloudAuth.test_token_file_permissions`. Co-authored-by: refraction-ray <35157286+refraction-ray@users.noreply.github.com>
|
make check_all.sh pass |
I have addressed the formatting issues in |
- Applied black formatting to `tests/test_cloud.py`. - Fixed pylint import order warnings. Co-authored-by: refraction-ray <35157286+refraction-ray@users.noreply.github.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
1 participant
This PR addresses a security vulnerability where the API token file (
~/.tc.auth.json) was created with default permissions (typically 0644), making it readable by other users on the system.Changes:
tensorcircuit/cloud/apis.pyto useos.openwithO_WRONLY | O_CREAT | O_TRUNCand mode0o600when writing the token file.os.chmod(authpath, 0o600)to ensure existing files are also secured.tests/test_auth_security.pythat verifies the file permissions are correctly set to0600on POSIX systems. The test usesunittest.mockto redirect file operations to a temporary directory, ensuring safety.Risk:
Verification:
tests/test_auth_security.pypasses on Linux.tests/test_backends.pypass.PR created automatically by Jules for task 12040696766820470383 started by @refraction-ray