Skip to content

Treat targets_modified overlay as data, not code#3488

Open
jmthomas wants to merge 4 commits into
mainfrom
exec
Open

Treat targets_modified overlay as data, not code#3488
jmthomas wants to merge 4 commits into
mainfrom
exec

Conversation

@jmthomas

Copy link
Copy Markdown
Member

The user-writable targets_modified/ overlay was executed as code via three routes reachable by non-admin users: table definitions (ERB + GENERIC eval), cmd/tlm definitions (overlaid by setup_targets), and Script Runner suite analysis. Read table/cmd-tlm definitions from the read-only targets/ tree only, require admin to write the cmd_tlm overlay (rejecting non-canonical keys), and gate suite analysis at the script_run tier.

🤖 Generated with Claude Code

The user-writable targets_modified/ overlay was executed as code via three
routes reachable by non-admin users: table definitions (ERB + GENERIC eval),
cmd/tlm definitions (overlaid by setup_targets), and Script Runner suite
analysis. Read table/cmd-tlm definitions from the read-only targets/ tree
only, require admin to write the cmd_tlm overlay (rejecting non-canonical
keys), and gate suite analysis at the script_run tier.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@jmthomas jmthomas requested review from clayandgen and ryanmelt June 18, 2026 02:35
@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.15686% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.22%. Comparing base (f29c725) to head (7a1538a).

Files with missing lines Patch % Lines
...r-api/app/controllers/running_script_controller.rb 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3488      +/-   ##
==========================================
- Coverage   79.25%   79.22%   -0.04%     
==========================================
  Files         689      689              
  Lines       57293    57323      +30     
  Branches      728      728              
==========================================
+ Hits        45406    45412       +6     
- Misses      11809    11833      +24     
  Partials       78       78              
Flag Coverage Δ
python 80.29% <ø> (+<0.01%) ⬆️
ruby-api 80.88% <91.66%> (-0.45%) ⬇️
ruby-backend 83.09% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread openc3-cosmos-script-runner-api/app/controllers/running_script_controller.rb Dismissed
Comment thread openc3-cosmos-cmd-tlm-api/app/controllers/storage_controller.rb Outdated
Comment thread openc3-cosmos-cmd-tlm-api/app/controllers/storage_controller.rb Outdated
Comment thread openc3-cosmos-script-runner-api/app/controllers/scripts_controller.rb Outdated
Comment thread openc3/lib/openc3/utilities/target_file.rb Outdated
Comment thread openc3/python/openc3/utilities/target_file.py Outdated
@jmthomas jmthomas requested a review from clayandgen June 18, 2026 15:47
jmthomas and others added 2 commits June 18, 2026 09:56
PR #3488 gated only the presigned-upload writer. Table.save/save_as/
generate/destroy reach the same targets_modified/<TARGET>/cmd_tlm/...
overlay via TargetFile.create, gated only by authorization('system'),
which under Enterprise RBAC every role down to Viewer holds. That overlay
is ERB-rendered (and eval'd via GENERIC_*_CONVERSION) as code at load, so
a non-admin could inject code. Add a shared authorize_overlay_write choke
point requiring admin for cmd_tlm paths, failing closed on non-canonical
names, mirroring storage_controller#non_admin_config_overlay_write?.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants