feat(cli): add /teleport command for portable session management#22585
feat(cli): add /teleport command for portable session management#22585mattKorwel wants to merge 3 commits intomainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Gemini CLI's utility by introducing a robust session teleportation feature. This allows users to seamlessly transfer their complete AI engineering workflow, including all contextual data, between various environments. The addition of encryption and cloud storage support provides flexibility and security, making it easier for developers to maintain continuity and collaborate across different machines and platforms. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new /teleport command for session portability, a significant and well-implemented feature that includes encryption, cloud storage integration, and comprehensive tests. However, several critical security vulnerabilities were identified in the implementation of TeleportService. These include command injection via option injection in system calls to tar and cloud CLI tools, path traversal vulnerabilities allowing arbitrary file read and write during the export process, and a lack of path validation for the --key-file option, which could lead to arbitrary file reads. Additionally, a high-severity bug in command argument handling was found. Addressing these issues by implementing strict input validation and using safe patterns for executing system commands is crucial for the security and reliability of this new feature.
| const result = spawnSync('tar', [ | ||
| '-czf', | ||
| tarPath, | ||
| '-C', | ||
| tempDir, | ||
| ...filesToInclude, | ||
| ]); |
There was a problem hiding this comment.
The TeleportService is vulnerable to command injection via option injection. User-supplied strings such as sessionId, outputPath, blobUri, and packagePathOrUri are passed directly as arguments to system commands (tar, gcloud, gsutil, aws) via spawnSync. Since these inputs are not validated to ensure they do not start with a hyphen (-), an attacker can provide inputs that are interpreted as command-line options. For example, providing --checkpoint-action=exec=id as an outputPath to tar can lead to arbitrary command execution.
To remediate this, use the -- separator in spawnSync calls to explicitly separate options from positional arguments, and validate that user-supplied paths or identifiers do not start with a hyphen.
| const result = spawnSync('tar', ['-xzf', extractionPath, '-C', tempDir]); | ||
|
|
||
| if (result.status !== 0) { | ||
| throw new Error( | ||
| `Failed to extract tarball: ${result.stderr.toString()}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The tar extraction process is vulnerable to a path traversal attack using symbolic links. A malicious archive could contain a symlink pointing to a sensitive location outside the intended extraction directory (e.g., a symlink a -> /). If the archive then contains a file like a/etc/passwd, the tar command could overwrite critical system files. The current check for .. in filenames is insufficient to prevent this attack vector.
To mitigate this, consider using a secure Node.js library like node-tar that offers options to prevent symlink traversal, or add platform-specific flags to the tar command to disable following symlinks.
References
- This comment describes a path traversal vulnerability during
tarextraction via symbolic links, which is a form of insecure file system operation involving potentially untrusted paths, aligning with the rule to sanitize user-provided file paths to prevent path traversal.
| const sessionDir = path.join(tempDir, sessionId); | ||
| if (fs.existsSync(sessionDir)) { | ||
| filesToInclude.push(sessionId); | ||
| } |
There was a problem hiding this comment.
The exportSession method is vulnerable to path traversal. The sessionId and outputPath arguments are used to construct file paths without proper validation or sanitization.
- The
sessionIdis used inpath.join(tempDir, sessionId)(line 78). If an attacker provides asessionIdlike../../../../etc/passwd, thetarcommand will include that file in the exported archive, leading to arbitrary file read. - The
outputPathis used as the destination for thetarcommand andfs.writeFileSync. An attacker can provide a path like../../.bashrcto overwrite arbitrary files on the system.
To remediate this, sanitize sessionId and outputPath using path.basename() or validate them against an allow-list of characters.
References
- This comment highlights a path traversal vulnerability where user-supplied
sessionIdandoutputPathare used in file system operations without proper sanitization, aligning with the rule to sanitize user-provided file paths to prevent such vulnerabilities.
| // 2. Check key file | ||
| if (keyFilePath) { | ||
| if (fs.existsSync(keyFilePath)) { | ||
| return fs.readFileSync(keyFilePath, 'utf8').trim(); |
There was a problem hiding this comment.
The getSecret function reads the content of a file specified by the user via the --key-file flag using fs.readFileSync(keyFilePath, 'utf8'). There is no validation on keyFilePath, allowing an attacker to read arbitrary files from the system. While the content is used as an encryption secret, this could be used to probe for the existence of files or potentially leak their contents if the export process is further manipulated.
To remediate this, validate that keyFilePath points to a regular file within an expected directory and is not a sensitive system file.
References
- This comment identifies a vulnerability where a user-provided
keyFilePathis used to read file content without validation, directly relating to the rule requiring sanitization of user-provided file paths in file system operations to prevent path traversal.
| keyFilePath = parts[i + 1]; | ||
| i++; |
There was a problem hiding this comment.
The --key-file argument is not validated to ensure a path is provided. If a user runs /teleport export --key-file without a subsequent path, the command unexpectedly falls back to an interactive prompt for the secret instead of failing with an error. This is inconsistent with the behavior of the --blob flag and can lead to confusing behavior for the user.
keyFilePath = parts[i + 1];
if (!keyFilePath || keyFilePath.startsWith('--')) {
return {
type: 'message',
messageType: 'error',
content: 'Please provide a path after the --key-file flag.',
};
}
i++;| keyFilePath = parts[i + 1]; | ||
| i++; |
There was a problem hiding this comment.
Similar to the export command, the --key-file argument in the import command is not validated. If the flag is used without a path, it will incorrectly fall back to an interactive prompt instead of erroring. This should be corrected to provide clear feedback to the user and maintain consistency with other flags.
keyFilePath = parts[i + 1];
if (!keyFilePath || keyFilePath.startsWith('--')) {
return {
type: 'message',
messageType: 'error',
content: 'Please provide a path after the --key-file flag.',
};
}
i++;|
Size Change: +16.1 kB (+0.06%) Total Size: 26.1 MB
ℹ️ View Unchanged
|
mattKorwel
left a comment
There was a problem hiding this comment.
🤖 Automated Review Assessment (via Gemini CLI)
Functional verification and behavioral analysis for this PR have identified a regression in the diagnostic pipeline alongside successful functionality.
✅ Key Improvements
- Teleport Command: The session management logic for portable state export/import correctly functions in the terminal environment.
- Build & Review: The core build and automated review passed successfully.
❌ Regression: Diagnostic Pipeline
- Issue: Both CI Status and Local Diagnostics failed. This points to a regression in the code that is being caught by the linter or type-checker.
- Action Required: Please check for unused imports or type mismatches in the new
/teleportcommand implementation.
Recommendation: Changes requested to resolve the diagnostic failures.
Summary
This PR adds the
/teleportcommand to Gemini CLI, allowing users to move active AI engineering sessions between different machines (e.g., from a local laptop to a remote server, or to a desktop machine when not using the laptop).Details
Unlike
/resume share, which only exports a chat transcript,/teleportpackages the entire workspace state, including:Security
--secret(interactive prompt) or--key-file.Transport
.tar.gzfiles.gcloud,gsutil,aws).Related Issues
None.
How to Validate
/teleport exportin a session./teleport import ./gemini-session-XXX.tar.gzon a machine with the same project./resume <id>that history and plans are restored./teleport export --secret./teleport import --secret ./encrypted.tar.gz./teleport export --blob gs://your-bucket/path./teleport import gs://your-bucket/path.npm test -w @google/gemini-cli-core -- src/services/teleportService.test.tsnpm test -w @google/gemini-cli -- src/ui/commands/teleportCommand.test.tsnpx vitest run integration-tests/teleport.test.tsPre-Merge Checklist