fix(places): follow-ups from the review of the schemeless website fix

create_place_accommodation refused the whole booking again once its website
went through the place contract, for '' and free text as well, which it always
took. It keeps its old input schema now and leaves a website off when the
contract refuses it, so a bare host still gains https and a script link is
still not stored.

normalizePlaceWebsite keeps an http or https value whenever a browser opens it,
the way the write contract does, so a plugin or source answering with an
intranet host, an IPv6 literal or a trailing dot keeps its website. The dot
check only tells a bare host from a word. A details row cached before the fix
(a week for Google and Amap, until a refresh for an expanded one) hands its
website out through the same helper.

Tests: SHARED-WEBSITE-007 adjusted, SHARED-WEBSITE-013 and 014,
MCP-ACCOM-2483-01 narrowed, MCP-ACCOM-2483-02, MAPS-2483-06 and 07, AMAP-079,
PLUGIN-SEARCH-2483-02.

server/src/nest/accommodations/accommodations.mcp.ts

@@ -93,8 +93,7 @@ export class AccommodationsMcp {
       google_ftid: z.string().optional().describe('Google Maps feature ID from search_place — enables direct Google Maps links'),
       osm_id: z.string().optional().describe('OpenStreetMap ID from search_place (e.g. "way:12345")'),
       place_notes: z.string().max(2000).optional().describe('Notes for the place'),
-      // The place contract create_place uses: a bare host gains https (#2483).
-      website: placeWebsiteSchema.optional(),
+      website: z.string().max(500).optional(),
       phone: z.string().max(50).optional(),
       start_day_id: z.number().int().positive().describe('Check-in day ID'),
       end_day_id: z.number().int().positive().describe('Check-out day ID'),
@@ -124,9 +123,13 @@ export class AccommodationsMcp {
     if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied();
     const dayErrors = this.accommodations.validateAccommodationRefs(tripId, undefined, start_day_id, end_day_id);
     if (dayErrors.length > 0) return { content: [{ type: 'text' as const, text: dayErrors.map(e => e.message).join(', ') }], isError: true };
+    // The website takes the place contract create_place uses, so a bare host
+    // gains https (#2483). A value that contract refuses is left off instead of
+    // failing the booking: this tool always took any text here, '' as none.
+    const site = website ? placeWebsiteSchema.safeParse(website) : null;
     try {
       const result = this.db.transaction(() => {
-        const place = this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, notes: place_notes, website, phone, price, currency });
+        const place = this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, notes: place_notes, website: site?.success ? site.data : undefined, phone, price, currency });
         const { accommodation, mirror } = this.accommodations.createAccommodation(tripId, { place_id: place.id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes: accommodation_notes });
         return { place, accommodation, mirror };
       });

server/src/nest/maps/maps.service.ts

@@ -672,6 +672,17 @@ export const PLACES_PROVIDER_SETTING = 'places_provider';
  */
 export const PLACES_GOOGLE_ONLY_SETTING = 'places_google_only';
 
+/**
+ * A details row as the cache holds it. A row written before #2483 still has a
+ * source's website as it came, `www.hotel.cn` from Amap for one, and keeps for a
+ * week (an expanded one until a refresh), so the website is normalized on the
+ * way out of the cache as well as on the way in.
+ */
+function cachedDetails(payload: string): Record<string, unknown> | null {
+  const place = JSON.parse(payload) as Record<string, unknown> | null;
+  return place && 'website' in place ? { ...place, website: normalizePlaceWebsite(place.website) } : place;
+}
+
 /**
  * Whoever holds the keyed slot beside the index for one request: Google's
  * credential, an Amap provider, or nobody (the OpenStreetMap stack alone).
@@ -2532,7 +2543,7 @@ export class MapsService {
       placeId,
       langKey,
     );
-    if (cached && Date.now() - cached.fetched_at < DETAILS_TTL) return { place: JSON.parse(cached.payload_json) };
+    if (cached && Date.now() - cached.fetched_at < DETAILS_TTL) return { place: cachedDetails(cached.payload_json) };
 
     // Closes the autocomplete session this lookup belongs to, so Google bills
     // the search once instead of per keystroke. A cache hit above never reaches
@@ -2625,7 +2636,7 @@ export class MapsService {
       placeId,
       langKey,
     );
-    if (cached && Date.now() - cached.fetched_at < DETAILS_TTL) return { place: JSON.parse(cached.payload_json) };
+    if (cached && Date.now() - cached.fetched_at < DETAILS_TTL) return { place: cachedDetails(cached.payload_json) };
 
     const place = await provider.placeDetails(placeId, lang);
     if (!place) return { place: null };
@@ -2681,7 +2692,7 @@ export class MapsService {
         placeId,
         langKey,
       );
-      if (cached) return { place: JSON.parse(cached.payload_json) };
+      if (cached) return { place: cachedDetails(cached.payload_json) };
     }
 
     const response = await googleFetch(

server/tests/unit/mcp/tools-days-accommodations.test.ts

@@ -337,7 +337,7 @@ describe('Tool: delete_accommodation', () => {
 describe('Tool: create_place_accommodation', () => {
   // #2483: the place it creates takes its website through the same contract as
   // create_place, so a bare host from search_place lands as https.
-  it('MCP-ACCOM-2483-01: a website without a scheme is stored with https, a script link is refused', async () => {
+  it('MCP-ACCOM-2483-01: a website without a scheme is stored with https', async () => {
     const { user } = createUser(testDb);
     const trip = createTrip(testDb, user.id);
     const day = createDay(testDb, trip.id);
@@ -347,14 +347,29 @@ describe('Tool: create_place_accommodation', () => {
         arguments: { tripId: trip.id, name: 'Hôtel du Faouët', start_day_id: day.id, end_day_id: day.id, website: 'www.hotel-faouet.example' },
       })) as { place: { id: number } };
       expect(testDb.prepare('SELECT website FROM places WHERE id = ?').get(data.place.id)).toEqual({ website: 'https://www.hotel-faouet.example' });
+    });
+  });
 
-      const refused = await h.client.callTool({
+  // The tool always took any text as the website. It still does, so no call that
+  // went through before fails now; a value the place contract refuses is left
+  // off rather than stored, and '' stays "no website" as it is on REST.
+  it('MCP-ACCOM-2483-02: a website that is empty, free text or a script link books the stay without one', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const day = createDay(testDb, trip.id);
+    await withHarness(user.id, async (h) => {
+      for (const [name, website] of [['Empty', ''], ['Words', 'ask at the front desk'], ['Hostile', 'javascript:alert(1)']]) {
+        const result = await h.client.callTool({
           name: 'create_place_accommodation',
-        arguments: { tripId: trip.id, name: 'Hostile', start_day_id: day.id, end_day_id: day.id, website: 'javascript:alert(1)' },
+          arguments: { tripId: trip.id, name, start_day_id: day.id, end_day_id: day.id, website },
         });
-      expect(refused.isError).toBe(true);
+        expect(result.isError, name).toBeFalsy();
+        const data = parseToolResult(result) as { place: { id: number; website: unknown }; accommodation: { id: number } };
+        expect(data.place.website, name).toBeNull();
+        expect(data.accommodation.id, name).toBeGreaterThan(0);
+        expect(testDb.prepare('SELECT website FROM places WHERE id = ?').get(data.place.id), name).toEqual({ website: null });
+      }
     });
-    expect(testDb.prepare("SELECT COUNT(*) AS n FROM places WHERE trip_id = ? AND name = 'Hostile'").get(trip.id)).toEqual({ n: 0 });
   });
 
   it('creates the place and the accommodation atomically and broadcasts both', async () => {

server/tests/unit/nest/maps.amap.test.ts

@@ -760,6 +760,22 @@ describe('MapsService.keyedProvider', () => {
     expect(place!.source).toBe('amap');
     expect(calledUrl()).not.toContain('overpass');
   });
+
+  // #2483: a details row cached before the fix holds Amap's website as it came,
+  // often a bare host, and keeps for a week.
+  it('AMAP-079: a cached details row with a bare website is served with https', async () => {
+    mockProviderGet.mockReturnValue({ value: 'amap' });
+    const row = { payload_json: JSON.stringify({ name: '旧酒店', website: 'www.hotel.cn', source: 'amap' }), fetched_at: Date.now() };
+    // The cache lookup binds the place id, the key lookup the user id.
+    mockDbGet.mockImplementation((...args: unknown[]) =>
+      args[0] === 'amap:B7' ? row : { maps_api_key: null, amap_api_key: 'akey' },
+    );
+    const fetchMock = vi.fn();
+    vi.stubGlobal('fetch', fetchMock);
+    const { place } = await svc.getPlaceDetails(1, 'amap:B7');
+    expect(place).toMatchObject({ name: '旧酒店', website: 'https://www.hotel.cn' });
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
 });
 
 describe('MapsService with Amap in the keyed slot', () => {

server/tests/unit/nest/maps.service.test.ts

@@ -3587,4 +3587,36 @@ describe('websites from the map sources (#2483)', () => {
     const place = (await svc.getPlaceDetailsExpanded(1, 'ChIJWeb-expanded')).place as { website: unknown };
     expect(place.website).toBeNull();
   });
+
+  // A details row cached before the fix still holds the website as the source
+  // sent it, and keeps for a week; an expanded row keeps until a refresh. The
+  // cache hands it out through the same helper.
+  const cachedRow = (place: Record<string, unknown>) => ({ payload_json: JSON.stringify(place), fetched_at: Date.now() });
+
+  it('MAPS-2483-06: a cached details row with a bare website is served with https, without a Google call', async () => {
+    const fetchMock = vi.fn();
+    vi.stubGlobal('fetch', fetchMock);
+    mockDbGet
+      .mockReturnValueOnce({ maps_api_key: 'gkey' })
+      .mockReturnValueOnce(cachedRow({ google_place_id: 'ChIJOld', name: 'Chapelle', website: 'example.fr/visite' }));
+    const { place } = await svc.getPlaceDetails(1, 'ChIJOld');
+    expect(place).toMatchObject({ name: 'Chapelle', website: 'https://example.fr/visite' });
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
+
+  it('MAPS-2483-07: so is an expanded row, where a script link becomes null and a row without the field stays as it is', async () => {
+    const fetchMock = vi.fn();
+    vi.stubGlobal('fetch', fetchMock);
+    mockDbGet
+      .mockReturnValueOnce({ maps_api_key: 'gkey' })
+      .mockReturnValueOnce(cachedRow({ google_place_id: 'ChIJOldX', website: '//www.example.fr', reviews: [] }))
+      .mockReturnValueOnce({ maps_api_key: 'gkey' })
+      .mockReturnValueOnce(cachedRow({ google_place_id: 'ChIJOldY', website: 'javascript:alert(1)' }))
+      .mockReturnValueOnce({ maps_api_key: 'gkey' })
+      .mockReturnValueOnce(cachedRow({ google_place_id: 'ChIJOldZ', name: 'No site' }));
+    expect((await svc.getPlaceDetailsExpanded(1, 'ChIJOldX')).place).toMatchObject({ website: 'https://www.example.fr', reviews: [] });
+    expect((await svc.getPlaceDetailsExpanded(1, 'ChIJOldY')).place).toMatchObject({ website: null });
+    expect((await svc.getPlaceDetailsExpanded(1, 'ChIJOldZ')).place).toEqual({ google_place_id: 'ChIJOldZ', name: 'No site' });
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
 });

server/tests/unit/plugins/plugin-search.controller.test.ts

@@ -165,6 +165,17 @@ describe('PluginSearchController', () => {
     expect(places.map((p) => p.website)).toEqual(['https://www.ristorante.example/menu', 'https://ristorante.example', null]);
   });
 
+  // A plugin on the same network may answer with an intranet address. The old
+  // check kept every http(s) url a browser opens, and so does the helper.
+  it('PLUGIN-SEARCH-2483-02: an http(s) website keeps its host whatever it looks like', async () => {
+    const urls = ['http://localhost:8080/poi/1', 'http://intranet/poi/2', 'https://[2001:db8::1]/poi/3'];
+    const { c } = controller({
+      searchPlaces: vi.fn(async () => urls.map((website, i) => hit({ id: `h${i}`, website }))) as unknown as PluginHooks['searchPlaces'],
+    });
+    const { places } = await c.search('poi', undefined, undefined, undefined, undefined, req(5));
+    expect(places.map((p) => p.website)).toEqual(urls);
+  });
+
   it('caps one provider at 20 hits', async () => {
     const { c } = controller({
       searchPlaces: vi.fn(async () =>

shared/src/place/place-website.spec.ts

@@ -1,4 +1,5 @@
 import { PLACE_WEBSITE_MAX_LENGTH, normalizePlaceWebsite } from './place-website';
+import { placeWebsiteSchema } from './place.schema';
 
 import { describe, expect, it } from 'vitest';
 
@@ -66,8 +67,8 @@ describe('normalizePlaceWebsite (#2483)', () => {
       'example-.com',
       'example.com.',
       'https://',
-      'https://localhost:3000',
       'https://exa mple.com',
+      'http://%zz/',
     ]) {
       expect(normalizePlaceWebsite(value), value).toBeNull();
     }
@@ -103,4 +104,38 @@ describe('normalizePlaceWebsite (#2483)', () => {
     expect(normalizePlaceWebsite(42)).toBeNull();
     expect(normalizePlaceWebsite({ href: 'https://example.com' })).toBeNull();
   });
+
+  // The dot is how a bare host is told from a word. A value that names http(s)
+  // has settled that already, so it is kept whenever a browser opens it, the
+  // way the write contract keeps it: a plugin on the same network may answer
+  // with an intranet address.
+  it('SHARED-WEBSITE-013: an http(s) value the contract stores is kept, whatever its host looks like', () => {
+    for (const value of [
+      'https://localhost:3000',
+      'http://intranet/wiki',
+      'https://[2001:db8::1]:8443/x',
+      'http://[::1]/',
+      'https://example.com.',
+      'https://ex%41mple.com',
+    ]) {
+      expect(normalizePlaceWebsite(value), value).toBe(value);
+      expect(placeWebsiteSchema.safeParse(value).data, value).toBe(value);
+    }
+  });
+
+  it('SHARED-WEBSITE-014: a backslash never turns a value into another scheme or a bare host into a kept one', () => {
+    // A browser reads the backslash as a slash, so this opens localhost over
+    // https. It is kept like any other intranet address: the scheme is http(s).
+    expect(normalizePlaceWebsite('https://localhost\\@example.com')).toBe('https://localhost\\@example.com');
+    for (const value of [
+      'https:\\\\evil.example',
+      '\\\\evil.example',
+      '//\\evil.example',
+      'example.com\\@evil.example',
+      'intranet\\@example.com',
+      'javascript:1\\@example.com',
+    ]) {
+      expect(normalizePlaceWebsite(value), value).toBeNull();
+    }
+  });
 });

shared/src/place/place-website.ts

@@ -35,7 +35,8 @@ function authorityOf(rest: string): string {
 
 /**
  * A name with at least one dot and a top-level label that has a letter in it,
- * or an IPv4 address. "Chapelle", "localhost" and "3.5" are not websites.
+ * or an IPv4 address. Without a scheme, "Chapelle", "localhost" and "3.5" are
+ * not websites.
  */
 function isHostName(host: string): boolean {
   const labels = host.split('.');
@@ -55,10 +56,13 @@ function hasHost(authority: string): boolean {
 /**
  * The website a place gets from `value`, or null.
  *
- * Trimmed; http and https stay as they are; a bare host or a protocol-relative
- * `//host` gains https. Any other scheme (`javascript:`, `data:`, `mailto:`,
- * `ftp:`), free text, a host without a dot and anything a browser cannot
- * parse come back as null, as does a value longer than the contract allows.
+ * Trimmed; http and https stay as they are whenever a browser can open them,
+ * an intranet host or an IPv6 literal included, which is what the write
+ * contract keeps as well. A bare host or a protocol-relative `//host` gains
+ * https, and only there does the host need a dot: it is what tells a site
+ * from a word. Any other scheme (`javascript:`, `data:`, `mailto:`, `ftp:`),
+ * free text and anything a browser cannot parse come back as null, as does a
+ * value longer than the contract allows.
  */
 export function normalizePlaceWebsite(value: unknown): string | null {
   if (typeof value !== 'string') return null;
@@ -67,9 +71,6 @@ export function normalizePlaceWebsite(value: unknown): string | null {
 
   let url: string;
   if (HTTP_URL.test(text)) {
-    const authority = authorityOf(text.replace(HTTP_URL, ''));
-    // Credentials in front of the host are not part of the name being checked.
-    if (!hasHost(authority.slice(authority.lastIndexOf('@') + 1))) return null;
     url = text;
   } else if (text.startsWith('//')) {
     if (!hasHost(authorityOf(text.slice(2)))) return null;