Skip to content

feat(storage): tighten VikingFS encryption-at-rest integration#1556

Open
yeyitech wants to merge 2 commits intovolcengine:mainfrom
yeyitech:feat/rfc379-encryption-vikingfs-phase1
Open

feat(storage): tighten VikingFS encryption-at-rest integration#1556
yeyitech wants to merge 2 commits intovolcengine:mainfrom
yeyitech:feat/rfc379-encryption-vikingfs-phase1

Conversation

@yeyitech
Copy link
Copy Markdown
Contributor

@yeyitech yeyitech commented Apr 17, 2026

Summary

  • unify VikingFS helper read paths and helper write paths around shared encrypted raw-byte helpers
  • ensure helper-path reads decrypt transparently while stored AGFS bytes remain ciphertext
  • add focused VikingFS tests that prove helper writes do not persist plaintext while helper reads still return plaintext

Testing

  • python -m compileall openviking/storage/viking_fs.py tests/storage/test_vikingfs_encryption_helpers.py
  • PYTHONPATH=. PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 pytest --noconftest -o addopts='' tests/storage/test_vikingfs_encryption_helpers.py

Scope

This PR intentionally stays inside VikingFS helper-path encryption behavior. It does not change crypto providers or broaden the encryption architecture.

Environment note

Broader repo pytest entrypoints remain blocked in this environment because tests/conftest.py imports optional Volcengine SDK modules that are not installed here.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Codex seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 92
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Exception Handling Regression Risk

The append_file method replaced explicit handling of AGFSHTTPError (with 404 check) with a generic Exception catch using is_not_found_error. If is_not_found_error does not correctly identify AGFSHTTPError with status code 404, this could change behavior for missing files.

except Exception as exc:
    if not is_not_found_error(exc):
        raise
    pass

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants