diff --git a/acqui/basket.pl b/acqui/basket.pl index af35330acd..8f49543680 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -688,8 +688,9 @@ sub get_order_infos { } # Format servicing instruction for display + # Parts are passed to the template as plain text and HTML-escaped there ([% part | html %]), + # rather than being joined into a raw HTML string here (see bug 30144 security follow-up). if ( $line{servicing_instruction} && $edifact_si_map ) { - my $si_display = ''; eval { my $groups = decode_json( $line{servicing_instruction} ); if ( ref $groups eq 'ARRAY' && @$groups ) { @@ -730,13 +731,12 @@ sub get_order_infos { } } } - $si_display = join( '
', @display_parts ) if @display_parts; + $line{servicing_instruction_display_parts} = \@display_parts if @display_parts; } }; if ($@) { Koha::Logger->get->warn("Failed to parse servicing_instruction JSON: $@"); } - $line{servicing_instruction_display} = $si_display; } return \%line; diff --git a/acqui/neworderempty.pl b/acqui/neworderempty.pl index 5e7abb27b2..8db1e0a5a1 100755 --- a/acqui/neworderempty.pl +++ b/acqui/neworderempty.pl @@ -452,6 +452,10 @@ my @servicing_instruction_authorised_values = Koha::AuthorisedValues->search( { order_by => 'lib' } )->as_list; +# Encode as JSON for safe embedding in a hidden form field (avoids raw interpolation into inline JS) +my $servicing_instruction_avs_json = encode_json( + [ map { { value => $_->authorised_value, label => $_->lib } } @servicing_instruction_authorised_values ] ); + # Parse servicing instruction JSON into array of groups my $servicing_instruction_groups = []; my $servicing_instruction_groups_json = '[]'; @@ -483,28 +487,28 @@ $template->param( closedate => $basket->{'closedate'}, # order details - suggestion => $suggestion, - biblionumber => $biblionumber, - uncertainprice => $data->{'uncertainprice'}, - discount_2dp => sprintf( "%.2f", $bookseller->discount ), # for display - discount => $bookseller->discount, - orderdiscount_2dp => sprintf( "%.2f", $data->{'discount'} || 0 ), - orderdiscount => $data->{'discount'}, - order_internalnote => $data->{'order_internalnote'}, - order_vendornote => $data->{'order_vendornote'}, - servicing_instruction_groups => $servicing_instruction_groups, - servicing_instruction_groups_json => $servicing_instruction_groups_json, - servicing_instruction_authorised_values => \@servicing_instruction_authorised_values, - listincgst => $bookseller->listincgst, - invoiceincgst => $bookseller->invoiceincgst, - cur_active_sym => $active_currency->symbol, - cur_active => $active_currency->currency, - currencies => Koha::Acquisition::Currencies->search, - currency => $data->{currency}, - vendor_currency => $bookseller->listprice, - orderexists => ( $new eq 'yes' ) ? 0 : 1, - title => $data->{'title'}, - author => $data->{'author'}, + suggestion => $suggestion, + biblionumber => $biblionumber, + uncertainprice => $data->{'uncertainprice'}, + discount_2dp => sprintf( "%.2f", $bookseller->discount ), # for display + discount => $bookseller->discount, + orderdiscount_2dp => sprintf( "%.2f", $data->{'discount'} || 0 ), + orderdiscount => $data->{'discount'}, + order_internalnote => $data->{'order_internalnote'}, + order_vendornote => $data->{'order_vendornote'}, + servicing_instruction_groups => $servicing_instruction_groups, + servicing_instruction_groups_json => $servicing_instruction_groups_json, + servicing_instruction_avs_json => $servicing_instruction_avs_json, + listincgst => $bookseller->listincgst, + invoiceincgst => $bookseller->invoiceincgst, + cur_active_sym => $active_currency->symbol, + cur_active => $active_currency->currency, + currencies => Koha::Acquisition::Currencies->search, + currency => $data->{currency}, + vendor_currency => $bookseller->listprice, + orderexists => ( $new eq 'yes' ) ? 0 : 1, + title => $data->{'title'}, + author => $data->{'author'}, publicationyear => $data->{'publicationyear'} ? $data->{'publicationyear'} : $data->{'copyrightdate'}, editionstatement => $data->{'editionstatement'}, budget_loop => $budget_loop, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt index 59bdfe4c29..d212532cd9 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt @@ -932,7 +932,11 @@ [% books_loo.invoice_object.invoicenumber | html %] [% END %] - [% books_loo.servicing_instruction_display | $raw %] + + [% FOREACH part IN books_loo.servicing_instruction_display_parts %] + [% part | html %][% UNLESS loop.last %]
[% END %] + [% END %] + [% IF Koha.Preference('EDIFACT') && ediaccount %] [% books_loo.suppliers_report | html %] [% ELSE %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt index 5c6b018c24..95d5011fdc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt @@ -239,19 +239,25 @@ // Servicing Instructions Management var servicingInstructionGroups = []; - var servicingInstructionAVs = [ - [% FOREACH av IN servicing_instruction_authorised_values %] - { value: '[% av.authorised_value | html %]', label: '[% av.lib | html %]' }, - [% END %] - ]; + var servicingInstructionAVs = []; function initServicingInstructions() { - // Load existing data from template + // Load authorised values and existing data from hidden fields rather than + // interpolating them directly into inline JS, so free-text values can't break + // out of a JS string literal (see bug 30144 security follow-up). try { - var existingData = JSON.parse('[% servicing_instruction_groups_json | $raw %]'); + var avsData = JSON.parse($("#servicing_instruction_avs_json").val() || "[]"); + servicingInstructionAVs = Array.isArray(avsData) ? avsData : []; + } catch (e) { + console.error("Failed to parse servicing instruction authorised values:", e); + servicingInstructionAVs = []; + } + + try { + var existingData = JSON.parse($("#servicing_instruction_groups_json").val() || "[]"); servicingInstructionGroups = Array.isArray(existingData) ? existingData : []; } catch (e) { - console.error('Failed to parse servicing instruction groups:', e); + console.error("Failed to parse servicing instruction groups:", e); servicingInstructionGroups = []; } @@ -259,12 +265,12 @@ updateJSON(); // Add group button handler - $('#add_servicing_group').on('click', function() { + $("#add_servicing_group").on("click", function () { addServicingGroup(); }); // Form submission handler - update JSON before submit - $('form').on('submit', function() { + $("form").on("submit", function () { updateJSON(); }); } @@ -286,23 +292,27 @@ // Validate: max 2 instructions per group if (group.length >= 2) { - alert('Each service group can contain at most 2 instructions (one LVC and/or one LVT).'); + alert("Each service group can contain at most 2 instructions (one LVC and/or one LVT)."); return; } - if (!type) type = 'LVC'; // Default to LVC (coded) - if (!value) value = ''; + if (!type) type = "LVC"; // Default to LVC (coded) + if (!value) value = ""; // Check if this type already exists in the group - var hasLVC = group.some(function(inst) { return inst.type === 'LVC'; }); - var hasLVT = group.some(function(inst) { return inst.type === 'LVT'; }); + var hasLVC = group.some(function (inst) { + return inst.type === "LVC"; + }); + var hasLVT = group.some(function (inst) { + return inst.type === "LVT"; + }); - if (type === 'LVC' && hasLVC) { - alert('This group already has an LVC instruction. Each group can have only one LVC.'); + if (type === "LVC" && hasLVC) { + alert("This group already has an LVC instruction. Each group can have only one LVC."); return; } - if (type === 'LVT' && hasLVT) { - alert('This group already has an LVT instruction. Each group can have only one LVT.'); + if (type === "LVT" && hasLVT) { + alert("This group already has an LVT instruction. Each group can have only one LVT."); return; } @@ -323,7 +333,7 @@ // Check if changing type would create a duplicate for (var i = 0; i < group.length; i++) { if (i !== instructionIndex && group[i].type === type) { - alert('This group already has a ' + type + ' instruction. Each group can have only one of each type.'); + alert("This group already has a " + type + " instruction. Each group can have only one of each type."); renderServicingInstructions(); // Re-render to reset the select return; } @@ -334,7 +344,7 @@ } function renderServicingInstructions() { - var container = $('#servicing_instructions_container'); + var container = $("#servicing_instructions_container"); container.empty(); if (servicingInstructionGroups.length === 0) { @@ -342,22 +352,22 @@ return; } - servicingInstructionGroups.forEach(function(group, groupIndex) { + servicingInstructionGroups.forEach(function (group, groupIndex) { var groupDiv = $('
'); groupDiv.css({ - 'border': '1px solid #ddd', - 'padding': '10px', - 'margin-bottom': '10px', - 'background': '#f9f9f9', - 'border-radius': '4px' + border: "1px solid #ddd", + padding: "10px", + "margin-bottom": "10px", + background: "#f9f9f9", + "border-radius": "4px", }); var groupHeader = $('
'); - groupHeader.append('Service Group ' + (groupIndex + 1) + ' '); + groupHeader.append("Service Group " + (groupIndex + 1) + " "); groupHeader.append( - $('') - .on('click', function() { removeServicingGroup(groupIndex); }) + $('').on("click", function () { + removeServicingGroup(groupIndex); + }) ); groupDiv.append(groupHeader); @@ -365,13 +375,13 @@ groupDiv.append('
Empty group
'); } - group.forEach(function(instruction, instructionIndex) { + group.forEach(function (instruction, instructionIndex) { var instructionDiv = $('
'); var typeSelect = $('') - .append('') - .append('') - .on('change', function() { + .append('") + .append('") + .on("change", function () { var newType = $(this).val(); var valueInput = $(this).next(); updateInstruction(groupIndex, instructionIndex, newType, valueInput.val()); @@ -379,29 +389,28 @@ }); var valueInput; - if (instruction.type === 'LVC' && servicingInstructionAVs.length > 0) { + if (instruction.type === "LVC" && servicingInstructionAVs.length > 0) { valueInput = $(''); valueInput.append(''); - servicingInstructionAVs.forEach(function(av) { + servicingInstructionAVs.forEach(function (av) { valueInput.append( - $('') - .attr('value', av.value) + $("") + .attr("value", av.value) .text(av.label) - .prop('selected', av.value === instruction.value) + .prop("selected", av.value === instruction.value) ); }); } else { - valueInput = $('') - .val(instruction.value); + valueInput = $('').val(instruction.value); } - valueInput.on('change', function() { + valueInput.on("change", function () { updateInstruction(groupIndex, instructionIndex, typeSelect.val(), $(this).val()); }); - var removeBtn = $('') - .on('click', function() { removeInstructionFromGroup(groupIndex, instructionIndex); }); + var removeBtn = $('').on("click", function () { + removeInstructionFromGroup(groupIndex, instructionIndex); + }); instructionDiv.append(typeSelect); instructionDiv.append(valueInput); @@ -410,32 +419,34 @@ }); // Determine what can be added to this group - var hasLVC = group.some(function(inst) { return inst.type === 'LVC'; }); - var hasLVT = group.some(function(inst) { return inst.type === 'LVT'; }); + var hasLVC = group.some(function (inst) { + return inst.type === "LVC"; + }); + var hasLVT = group.some(function (inst) { + return inst.type === "LVT"; + }); var canAddMore = group.length < 2; if (canAddMore) { var btnText = ' Add instruction'; if (hasLVC && !hasLVT) { - btnText += ' (LVT only)'; + btnText += " (LVT only)"; } else if (hasLVT && !hasLVC) { - btnText += ' (LVC only)'; + btnText += " (LVC only)"; } else if (group.length === 0) { - btnText += ' to this group'; + btnText += " to this group"; } - var addInstructionBtn = $('') - .on('click', function() { - // Determine default type based on what's already in the group - var defaultType = 'LVC'; - if (hasLVC && !hasLVT) defaultType = 'LVT'; - else if (hasLVT && !hasLVC) defaultType = 'LVC'; - addInstructionToGroup(groupIndex, defaultType); - }); + var addInstructionBtn = $('").on("click", function () { + // Determine default type based on what's already in the group + var defaultType = "LVC"; + if (hasLVC && !hasLVT) defaultType = "LVT"; + else if (hasLVT && !hasLVC) defaultType = "LVC"; + addInstructionToGroup(groupIndex, defaultType); + }); groupDiv.append(addInstructionBtn); } else { - var maxMsg = $('
' + - 'Maximum 2 instructions per group (one LVC + one LVT)
'); + var maxMsg = $('
' + "Maximum 2 instructions per group (one LVC + one LVT)
"); groupDiv.append(maxMsg); } @@ -446,17 +457,17 @@ function updateJSON() { // Filter out empty groups and instructions with empty values var filtered = servicingInstructionGroups - .map(function(group) { - return group.filter(function(instruction) { - return instruction.value && instruction.value.trim() !== ''; + .map(function (group) { + return group.filter(function (instruction) { + return instruction.value && instruction.value.trim() !== ""; }); }) - .filter(function(group) { + .filter(function (group) { return group.length > 0; }); var json = JSON.stringify(filtered); - $('#servicing_instruction_groups_json').val(json); + $("#servicing_instruction_groups_json").val(json); } [% Asset.css("css/addbiblio.css") | $raw %] @@ -956,7 +967,8 @@ - + +
EDIFACT servicing instructions (GIR LVT/LVC). Each group can contain multiple coded (LVC) and/or free-text (LVT) instructions that work together.