feat(tvc): debug mode in app and deployment intents#132
Conversation
fab61ef to
2fb1888
Compare
--dangerous_deploy_insecure to create deployment |
reminder to self to regenerate protos from |
richardpringle
left a comment
There was a problem hiding this comment.
Just want you to change the doc warning and the Cargo.lock change (unless it's needed for some reason)
Everything else is optional
| pub async fn run(args: Args) -> Result<()> { | ||
| let app_config = load_or_fill_app_config(&args.config_file).await?; | ||
| let mut app_config = load_or_fill_app_config(&args.config_file).await?; | ||
| apply_overrides(&mut app_config, &args); |
There was a problem hiding this comment.
bit of a nit
I'm not sure you need this extra function. I understand why you would want to have it if you had more than one override, but I think we should put that in there if and only if we have more overrides.
In any case, I think it's better to keep the newly constructed app_config immutable and apply any overrides inside the apply_overrides function.
There was a problem hiding this comment.
I like the clarity and readability of apply_overrides() even though it is overkill for one update. i've changed it to be a one-liner with the resulting config immutable, which I think is what you meant
| fn dangerous_debug_mode_flag_enables_debug_mode() { | ||
| let file = write_config(&file_config()); // file has debug_mode = false | ||
| let args = Args { | ||
| config_file: Some(file.path().to_path_buf()), | ||
| dangerous_deploy_debug_mode: true, | ||
| ..empty_args() | ||
| }; | ||
| let resolved = run_resolve(&args).unwrap(); | ||
| assert!(resolved.debug_mode); | ||
| } |
There was a problem hiding this comment.
Don't need to change this, but this is why I don't like to hide logic inside functions. resolve_deploy_config Should just take Option<DeployConfig> as an argument which is the result of read_config_file. When we flatten out our logic, we don't have to do things like write an actual file and read it in just to see if the file that's read in is properly overwritten.
There was a problem hiding this comment.
agree that this should be cleaned up and flattened into a pipeline of more pure functions. Won't do in this PR
| checksum = "f60246a4944f24f6e018aa17cdeffb7818b76356965d03b07d6a9886e8962185" | ||
| dependencies = [ | ||
| "cfg-if", | ||
| ] |
There was a problem hiding this comment.
Why did this change? I don't think there should be any Cargo.lock changes without Cargo.toml changes, right? At least they probably shouldn't be coupled with logic that doesn't use the changes.
There was a problem hiding this comment.
+1 for reverting if possible
There was a problem hiding this comment.
not sure why it changed, will attempt to revert
There was a problem hiding this comment.
Richard fixed it in #147, so I will rebase and this should go away
- matches work and protos in mono PR #6314
2fb1888 to
549b919
Compare
- PR comments
- PR comments
Addressed Richard's changes and fixed Cargo.lock drift with PR #148
There was a problem hiding this comment.
Pull request overview
This PR adds Rust SDK support for TVC debug-mode app/deployment opt-ins, aligning CLI/config handling and generated proto types with the server-side intent changes.
Changes:
- Adds app config and intent support for
enableDebugModeDeployments. - Renames deployment debug CLI/env opt-in to the dangerous debug-mode flag/env.
- Updates proto definitions and generated Rust client structs for the new TVC app field.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tvc/src/config/deploy.rs |
Changes deployment debugMode config to a defaulted boolean with warning docs. |
tvc/src/config/app.rs |
Adds app-level debug-mode deployment capability config. |
tvc/src/commands/deploy/create.rs |
Adds dangerous deployment debug flag handling and forwards it to create intent. |
tvc/src/commands/app/create.rs |
Adds dangerous app debug-capability flag handling and forwards it to app create intent. |
proto/immutable/activity/v1/activity.proto |
Adds enable_debug_mode_deployments to CreateTvcAppIntent. |
proto/external/data/v1/tvc.proto |
Adds enable_debug_mode_deployments to TvcApp. |
client/src/generated/immutable.activity.v1.rs |
Regenerates Rust intent struct with the new optional field. |
client/src/generated/external.data.v1.rs |
Regenerates Rust TVC app struct with the new field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// | ||
| /// Overrides `debugMode` in the config file when set. | ||
| #[arg(long, env = "TVC_DANGEROUS_DEPLOY_DEBUG_MODE")] | ||
| pub dangerous_deploy_debug_mode: bool, |
| /// attestation cannot succeed. Cannot be changed after app creation; setting | ||
| /// this true means the app's quorum key is considered permanently insecure. | ||
| #[arg(long, env = "TVC_DANGEROUS_ENABLE_DEBUG_MODE_DEPLOYMENTS")] | ||
| pub dangerous_enable_debug_mode_deployments: bool, |
Summary
Debug-mode deployments expose enclave logs and zero out attestation PCRs, permanently marking an app's quorum key insecure. To make that risk explicit, debug-mode deployment requires a two-step opt-in: an app-level flag set at creation, plus a per-deployment flag that the server only accepts for capable apps. Attempting a debug-mode deployment on an app that isn't opted in will error from
mono.This is implemented in
mono PR 6314.
This PR adds equivalent functionality to the Rust SDK with the updated protos/intents
tvc app createNew flag:
--dangerous-enable-debug-mode-deploymentsTVC_DANGEROUS_ENABLE_DEBUG_MODE_DEPLOYMENTSfalseNew JSON config field matching the field in the proto:
{ "enableDebugModeDeployments": false }Warning
Cannot be changed after the app is created. To move between debug-capable and non-debug, create a new app with a fresh quorum key.
tvc deploy create--debug-modewas previously a no-op, so this breaking change is fine.The server rejects debug-mode deployments for apps that weren't created with `--dangerous-enable-debug-mode-deployments`.
The JSON deploy config's `debugMode` field is now plain `bool` (defaults to `false` when omitted) rather than `bool | null`. Existing configs with `true`, `false`, or the field omitted keep working unchanged.
Override order
Both flags follow the existing
claporder:Proto
Synced
proto/external/data/v1/tvc.protoandproto/immutable/activity/v1/activity.protofrom mono PR #6314 to pick up the newenable_debug_mode_deploymentsfield onCreateTvcAppIntentandTvcApp.