Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions openc3-cosmos-cmd-tlm-api/app/controllers/storage_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,13 @@ def download_file
storage_type, storage_name = validate_storage_source
object_id = sanitize_path(params[:object_id])

# Guard against a missing / literal 'null' object_id (e.g. frontend sending
# an undefined log or report filename). Return 404 instead of a 500.
if object_id.empty? || object_id == 'null'
render json: { status: 'error', message: "Invalid object_id: #{params[:object_id]}" }, status: :not_found
return
end

# Check scope-based RBAC for bucket downloads
if storage_type == :bucket && params[:bucket] && bucket_requires_rbac?(params[:bucket])
unless authorize_bucket_path(params[:bucket], object_id)
Expand All @@ -323,6 +330,13 @@ def download_file
temp_path
end

# get_object returns nil for a missing key (never writes temp_path), and a
# volume file may not exist. Return 404 rather than letting File.read 500.
unless File.exist?(filename)
render json: { status: 'error', message: "File not found: #{params[:object_id]}" }, status: :not_found
return
end

file = File.read(filename, mode: 'rb')

# Check if CTRF conversion is requested
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@
allow(FileUtils).to receive(:mkdir_p)
allow(FileUtils).to receive(:rm_rf)
allow(File).to receive(:read).and_return("file content")
allow(File).to receive(:exist?).and_return(true)
allow(Base64).to receive(:encode64).and_return("encoded_content")

get :download_file, params: {bucket: "OPENC3_CONFIG_BUCKET", object_id: "file.txt", scope: "DEFAULT"}
Expand All @@ -184,6 +185,7 @@

it "downloads a file from a volume" do
allow(File).to receive(:read).and_return("file content")
allow(File).to receive(:exist?).and_return(true)
allow(Base64).to receive(:encode64).and_return("encoded_content")

get :download_file, params: {volume: "OPENC3_DATA_VOLUME", object_id: "file.txt", scope: "DEFAULT"}
Expand Down Expand Up @@ -296,6 +298,7 @@
allow(Dir).to receive(:mktmpdir).and_return("/tmp/dir")
allow(FileUtils).to receive(:mkdir_p)
allow(FileUtils).to receive(:rm_rf)
allow(File).to receive(:exist?).and_return(true)
end

it "converts ruby test reports to CTRF format" do
Expand Down Expand Up @@ -1151,6 +1154,7 @@
allow(FileUtils).to receive(:mkdir_p)
allow(FileUtils).to receive(:rm_rf)
allow(File).to receive(:read).and_return("file content")
allow(File).to receive(:exist?).and_return(true)

# Mock authorize to allow INST target with tlm permission
allow(controller).to receive(:authorize) do |args|
Expand All @@ -1175,6 +1179,7 @@
allow(FileUtils).to receive(:mkdir_p)
allow(FileUtils).to receive(:rm_rf)
allow(File).to receive(:read).and_return("file content")
allow(File).to receive(:exist?).and_return(true)

get :download_file, params: {bucket: "OPENC3_TOOLS_BUCKET", object_id: "tool/file.txt", scope: "DEFAULT"}
expect(response).to have_http_status(:ok)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,14 @@
color="primary"
density="comfortable"
icon="mdi-eye"
:disabled="!item.log"
@click="viewScriptLog(item, 'log')"
/>
<v-btn
color="primary"
density="comfortable"
icon="mdi-file-download-outline"
:disabled="downloadScript"
:disabled="downloadScript || !item.log"
:loading="downloadScript && downloadScript.name === item.name"
@click="downloadScriptLog(item, 'log')"
/>
Expand Down Expand Up @@ -513,16 +514,30 @@ async function viewScriptLog(script, type) {
dialogName.value = 'Log'
logUrl = script.log
}
const response = await Api.get(
`/openc3-api/storage/download_file/${encodeURIComponent(
logUrl,
)}?bucket=OPENC3_LOGS_BUCKET`,
)
const filenameParts = logUrl.split('/')
dialogFilename.value = filenameParts[filenameParts.length - 1]
// Decode Base64 string
dialogContent.value = window.atob(response.data.contents)
showDialog.value = true
if (!logUrl) {
notify.caution({
title: `No ${dialogName.value.toLowerCase()} available`,
body: `Script ${script.name} has no ${dialogName.value.toLowerCase()} file yet.`,
})
return
}
try {
const response = await Api.get(
`/openc3-api/storage/download_file/${encodeURIComponent(
logUrl,
)}?bucket=OPENC3_LOGS_BUCKET`,
)
const filenameParts = logUrl.split('/')
dialogFilename.value = filenameParts[filenameParts.length - 1]
// Decode Base64 string
dialogContent.value = window.atob(response.data.contents)
showDialog.value = true
} catch {
notify.caution({
title: `Unable to open ${dialogName.value.toLowerCase()} ${logUrl}`,
body: `You may be able to download this ${dialogName.value.toLowerCase()} manually from the 'logs' bucket at ${logUrl}`,
})
}
}

async function downloadScriptLog(script, type, format = 'text') {
Expand All @@ -534,6 +549,13 @@ async function downloadScriptLog(script, type, format = 'text') {
dialogName.value = 'Log'
logUrl = script.log
}
if (!logUrl) {
notify.caution({
title: `No ${dialogName.value.toLowerCase()} available`,
body: `Script ${script.name} has no ${dialogName.value.toLowerCase()} file yet.`,
})
return
}
downloadScript.value = script

try {
Expand Down
8 changes: 4 additions & 4 deletions openc3/python/test/script/test_api_shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ def test_waits_for_a_tgt_pkt_item(self):
result = wait("INST HEALTH_STATUS TEMP1 < 0", 0.01, 0.01) # Last param is polling rate
self.assertFalse(result)
self.assertIn(
"WAIT: INST HEALTH_STATUS TEMP1 < 0 failed with value == 10 after waiting 0.01",
"WAIT: INST HEALTH_STATUS TEMP1 < 0 failed with value == 10 after waiting",
stdout.getvalue(),
)

Expand All @@ -546,7 +546,7 @@ def test_waits_for_a_tgt_pkt_item(self):
result = wait("INST", "HEALTH_STATUS", "TEMP1", "== 0", 0.01, 0.01) # Last param is polling rate
self.assertFalse(result)
self.assertIn(
"WAIT: INST HEALTH_STATUS TEMP1 == 0 failed with value == 10 after waiting 0.01",
"WAIT: INST HEALTH_STATUS TEMP1 == 0 failed with value == 10 after waiting",
stdout.getvalue(),
)

Expand Down Expand Up @@ -600,7 +600,7 @@ def test_waits_for_a_value_to_be_within_a_tolerance(self):
result = wait_tolerance("INST HEALTH_STATUS TEMP2", 11, 0.1, 0.01)
self.assertFalse(result)
self.assertIn(
"WAIT: INST HEALTH_STATUS TEMP2 failed to be within range 10.9 to 11.1 with value == 10.5 after waiting 0.01",
"WAIT: INST HEALTH_STATUS TEMP2 failed to be within range 10.9 to 11.1 with value == 10.5 after waiting",
stdout.getvalue(),
)

Expand Down Expand Up @@ -764,7 +764,7 @@ def test_checks_a_telemetry_item_against_a_value(self):
)
with self.assertRaisesRegex(
CheckError,
"CHECK: INST HEALTH_STATUS TEMP1 > 100 failed with value == 10 after waiting 0.01",
"CHECK: INST HEALTH_STATUS TEMP1 > 100 failed with value == 10 after waiting",
):
wait_check("INST HEALTH_STATUS TEMP1 > 100", 0.01)

Expand Down
Loading